updates bug fix

README.md

@@ -92,7 +92,7 @@ A bar-style prompt showing current time, execution time of the last command
 | Function | Module | Description |
 | --- | --- | --- |
 | `busy` | fun | Monitor /dev/urandom for a hex pattern — look busy; the pattern can also be given as argument; runs until a key (or Ctrl-C) is pressed, needs a terminal |
-| `check_updates` | updates | Check whether a newer profile version is available online; when called with `-q` at startup a 3-second network timeout is applied so a slow or absent network never delays the prompt |
+| `check_updates` | updates | Check whether a newer profile version is available online; when called with `-q` at startup a 3-second network timeout is applied so a slow or absent network never delays the prompt; the published version is validated before use and an installed version more recent than the published one is not reported as outdated |
 | `clean` | filefct | Erase backup files in given directories, optionally recursive |
 | `conf_dump` | conf | Display the profile configuration file; `-s/--section NAME` restricts output to one section; `-a/--all` also lists commented-out keys (documented defaults, skipped when the key is set); an optional pattern argument filters keys by substring match; honours `NO_COLOR` |
 | `conf_save` | conf | Save or update a key=value pair in a configuration section: `conf_save <section> <key> <value>`; replaces an existing key in place or appends it to the section (creating the section if absent); works through a symlinked file and keeps its permissions; the value must be single-line and the running shell is not updated (applies at next load) |
@@ -134,7 +134,7 @@ A bar-style prompt showing current time, execution time of the last command
 | `ppg` | processes | Look for the given pattern in running processes |
 | `ppn` | processes | List processes whose command name (as shown by `ps -o comm`) is exactly the given one |
 | `ppu` | processes | List processes owned by a specific user |
-| `profile_upgrade` | updates | Upgrade profile to the latest version (git pull or archive) |
+| `profile_upgrade` | updates | Upgrade profile to the latest version: `git pull --ff-only` on a Git install, otherwise the downloaded (or `-f` local) archive, refused if it contains absolute or `..` paths; `-g` switches to a Git clone keeping `profile.conf` and custom themes; modules left over from an older version are listed, never deleted; `-n` shows what would be done |
 | `pwdscore` | pwd | Calculate the strength score of a given password |
 | `rain` | rain | Console screensaver with falling-rain effect (multiple color themes) |
 | `rainbow` | rain | Full-screen rainbow screensaver using only background colors, with horizontal color shifting |
@@ -328,7 +328,7 @@ change the default without having to pass flags every time.
 
 | Key | Default | Description |
 | --- | --- | --- |
-| `UPDT_DEFAULT_BRANCH` | `master` | Git branch used for update checks and `profile_upgrade`. Changing this value causes `profile_upgrade` to automatically switch the local checkout to the new branch on the next run and display a warning. |
+| `UPDT_DEFAULT_BRANCH` | `master` | Git branch used for update checks and `profile_upgrade` (letters, digits, `.`, `_`, `-`, `/`; anything else falls back to `master`). Changing this value causes `profile_upgrade` to automatically switch the local checkout to the new branch on the next run and display a warning. |
 
 ### 4.3. Locale shortcuts
 

doc/CHANGELOG.md

@@ -161,6 +161,17 @@ Versions follow `MAJOR.MINOR.PATCH-REVISION_STAGE_N` (e.g. `3.99.1-4_rc_1`).
   processed twice; `@cert-authority` lines are never touched. `ssr` ignored an
   empty `SSH_DEFAULT_OPT` (documented as "no option") and now rejects an
   option given in place of the server.
+- `updates.sh`: the published version file, read from the network, was displayed
+  without checking (escape sequences could reach the terminal at every
+  startup): it is now validated, and compared as a version (an installed
+  development version was reported as outdated); in quiet mode errors are really
+  silent; `profile_upgrade --switch-to-git` left the user's `profile.conf` and
+  custom themes in the backup directory (they are now copied to the new
+  clone); `git pull` could create merge commits or conflict markers in the
+  installed scripts (now `--ff-only`); the branch name is validated; archives
+  with absolute or `..` paths are refused and extracted without their owners;
+  modules removed upstream and still loaded are reported; git operations no
+  longer change the shell's directory stack.
 - `compress.sh`: option flags and loop variables no longer leak into the
   interactive shell.
 

profile.d/updates.sh

@@ -40,14 +40,49 @@ export BASE_URL="https://git.geoffray-levasseur.org/fatalerrors/profile"
 # UPDT_DEFAULT_BRANCH (if set in profile.conf) is already available.
 load_conf "updates"
 
+# ------------------------------------------------------------------------------
+# Check a branch name. It ends up in URLs and in git command lines, so it is
+# restricted to plain ref characters, without a leading "-" or "..".
+# Usage: _updt_valid_branch <name>
+_updt_valid_branch()
+{
+    [[ "$1" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ && "$1" != *..* ]]
+}
+
+if ! _updt_valid_branch "${UPDT_DEFAULT_BRANCH:-master}"; then
+    disp W "Invalid UPDT_DEFAULT_BRANCH '${UPDT_DEFAULT_BRANCH}', using master."
+    UPDT_DEFAULT_BRANCH=master
+fi
+
 export UPDT_DEFAULT_BRANCH="${UPDT_DEFAULT_BRANCH:-master}"
 export UPDT_URL="$BASE_URL/raw/branch/$UPDT_DEFAULT_BRANCH"
 export ARCH_URL="$BASE_URL/archive/$UPDT_DEFAULT_BRANCH.tar.gz"
 
+# ------------------------------------------------------------------------------
+# Tell whether the published version $2 is an update for the installed one $1.
+# Versions look like MAJOR.MINOR.PATCH[-REVISION[_STAGE_N]]. The base version
+# (before the first "-" or "_") decides, compared in version order; with the same
+# base, any difference (a new revision, a release candidate becoming final) is
+# an update. Returns 0 when it is, 1 otherwise (equal, or the installed version
+# is the more recent one, as on a development branch).
+# Usage: _updt_is_newer <installed> <published>
+_updt_is_newer()
+{
+    local cur="$1" new="$2" cur_base new_base
+    [[ "$cur" == "$new" ]] && return 1
+    cur_base="${cur%%[-_]*}"
+    new_base="${new%%[-_]*}"
+    [[ "$cur_base" == "$new_base" ]] && return 0
+    [[ "$(printf '%s\n%s\n' "$cur_base" "$new_base" | sort -V | tail -n 1)" == "$new_base" ]]
+}
+
 # ------------------------------------------------------------------------------
 # Check whether a newer profile version is available
 # Usage: check_updates [-q]
-# If -q is specified, the function will operate in quiet mode (internal use only)
+# If -q is specified, the function will operate in quiet mode (internal use
+# only): only the "update available" notice is displayed, errors are silent.
+# Return status: 0 up to date, 1 update available, 2 invalid option,
+# 4 temporary file error, 5 version unavailable or invalid.
 check_updates()
 {
     local quiet=0 result=5 PARSED
@@ -59,9 +94,8 @@ check_updates()
         disp E "Invalid options, use \"check_updates --help\" to display usage."
         return 2
     fi
-    local -a _pargs=()
+    # Restore the getopt-normalised (shell-quoted) argument list
-    readarray -td '' _pargs < <(printf '%s' "$PARSED" | xargs printf '%s\0')
+    eval set -- "$PARSED"
-    set -- "${_pargs[@]}"
 
     while true; do
         case "$1" in
@@ -87,8 +121,8 @@ check_updates()
 
     (( quiet != 1 )) && disp I "Checking for updates..."
 
-    vfile=$(mktemp /tmp/profile_version.XXXXXX) || {
+    vfile=$(mktemp "${TMPDIR:-/tmp}/profile_version.XXXXXX") || {
-        disp E "Failed to create a temporary file."
+        (( quiet != 1 )) && disp E "Failed to create a temporary file."
         return 4
     }
 
@@ -103,20 +137,30 @@ check_updates()
         return 5
     }
 
-    if [[ -s $vfile ]]; then
+    # First line only, and only a plausible version: the text comes from the
-        lastver=$(<"$vfile")
+    # network and is displayed by disp, which interprets backslash escapes (a
-        if [[ "$lastver" != "$PROFVERSION" ]]; then
+    # crafted file, or a captive portal page, could otherwise inject terminal
-            disp I "Installed: $PROFVERSION. Available: $lastver."
+    # control sequences at every shell startup).
-            (( quiet != 1 )) && disp I "You should upgrade when possible."
+    IFS= read -r lastver < "$vfile" || true
-            result=1
+    rm -f "$vfile"
-        else
+    lastver="${lastver%$'\r'}"
-            (( quiet != 1 )) && disp I "Your version is up-to-date."
+    lastver="${lastver#"${lastver%%[![:space:]]*}"}"
-            result=0
+    lastver="${lastver%"${lastver##*[![:space:]]}"}"
-        fi
+    if [[ ! "$lastver" =~ ^[0-9]+(\.[0-9]+)*([-_.+][0-9A-Za-z._+-]+)?$ ]]; then
-        rm -f "$vfile"
+        (( quiet != 1 )) && disp E "The downloaded version file is empty or invalid; unable to continue."
+        return 5
+    fi
+
+    if [[ "$lastver" == "${PROFVERSION:-}" ]]; then
+        (( quiet != 1 )) && disp I "Your version is up-to-date."
+        result=0
+    elif _updt_is_newer "${PROFVERSION:-}" "$lastver"; then
+        disp I "Installed: ${PROFVERSION:-unknown}. Available: $lastver."
+        (( quiet != 1 )) && disp I "You should upgrade when possible."
+        result=1
     else
-        rm -f "$vfile"
+        (( quiet != 1 )) && disp I "Installed version (${PROFVERSION:-}) is more recent than the published one ($lastver)."
-        disp E "Temporary file is unreadable; unable to continue."
+        result=0
     fi
 
     return $result
@@ -128,6 +172,9 @@ export -f check_updates
 # ------------------------------------------------------------------------------
 # Apply the available profile upgrade
 # Usage: profile_upgrade [options]
+# Return status: 0 done (or nothing to do), 1 refused/bad context, 2 git or
+# option error, 3 git/install error, 4 local archive or force error, 5
+# temporary directory, 6 download, 7 extraction, 8 archive content, 9 copy.
 profile_upgrade()
 {
     local PARSED
@@ -141,9 +188,8 @@ profile_upgrade()
         disp E "Invalid options, use \"profile_upgrade --help\" to display usage."
         return 2
     fi
-    local -a _pargs=()
+    # Restore the getopt-normalised (shell-quoted) argument list
-    readarray -td '' _pargs < <(printf '%s' "$PARSED" | xargs printf '%s\0')
+    eval set -- "$PARSED"
-    set -- "${_pargs[@]}"
     while true; do
         case "$1" in
             -h|--help)
@@ -157,7 +203,7 @@ profile_upgrade()
                 printf "\t-g, --switch-to-git\tReplace current install with a fresh Git clone\n"
                 printf "\t-n, --dry-run\t\tDisplay what would be done without changing anything\n"
                 printf "\t-F, --force\t\tDiscard local changes before upgrading\n\n"
-                printf "If the profile is installed from Git, the upgrade uses 'git pull'.\n"
+                printf "If the profile is installed from Git, the upgrade uses 'git pull --ff-only'.\n"
                 printf "Otherwise, it downloads or applies an archive and refreshes the files.\n"
                 return 0
                 ;;
@@ -197,6 +243,13 @@ profile_upgrade()
         esac
     done
 
+    # Target branch: explicit -b flag > config default > master
+    local target_branch="${branch:-${UPDT_DEFAULT_BRANCH:-master}}"
+    if ! _updt_valid_branch "$target_branch"; then
+        disp E "Invalid branch name '$target_branch'."
+        return 1
+    fi
+
     if (( ! use_archive && ! switch_to_git )); then
         check_updates -q
         check_rc=$?
@@ -224,17 +277,18 @@ profile_upgrade()
             disp E "Git is required to switch this install to a Git clone."
             return 3
         }
+        local backup_dir="${MYPATH}.pre-git.$$.bak"
         if (( dry_run )); then
-            disp I "[dry-run] rm -rf \"$MYPATH\"/.git"
+            disp I "[dry-run] mv \"$MYPATH\" \"$backup_dir\""
             disp I "[dry-run] git clone $BASE_URL \"$MYPATH\""
             [[ -n "$branch" ]] && disp I "[dry-run] git -C \"$MYPATH\" checkout $branch"
+            disp I "[dry-run] copy profile.conf and custom themes from \"$backup_dir\""
             return 0
         fi
 
         if [[ -d $MYPATH/.git ]]; then
             disp W "Git repository already present; no switch is needed."
         else
-            local backup_dir="${MYPATH}.pre-git.$$.bak"
             mv "$MYPATH" "$backup_dir" || {
                 disp E "Failed to move current install out of the way."
                 return 3
@@ -247,8 +301,18 @@ profile_upgrade()
             [[ -n "$branch" ]] && (
                 cd "$MYPATH" && git checkout "$branch"
             ) || true
+
+            # The personal files are not in the repository (profile.conf is
+            # git-ignored): without this the new install would start without
+            # the user's configuration, which would stay in the backup.
+            # Existing files are never overwritten.
+            [[ -f "$backup_dir/profile.conf" ]] && \
+                cp -an "$backup_dir/profile.conf" "$MYPATH/profile.conf"
+            if [[ -d "$backup_dir/profile.d/themes" && -d "$MYPATH/profile.d/themes" ]]; then
+                cp -an "$backup_dir/profile.d/themes/." "$MYPATH/profile.d/themes/"
+            fi
             disp I "Switched installation to Git source."
-            disp I "Previous install kept in $backup_dir."
+            disp I "Previous install kept in $backup_dir (profile.conf and custom themes were copied)."
             return 0
         fi
     fi
@@ -259,23 +323,18 @@ profile_upgrade()
             disp E "Git is required for this upgrade but is not available."
             return 3
         }
-        pushd "$MYPATH" >/dev/null || {
+        # git -C instead of cd: no directory change to undo on each exit path
-            disp E "Failed to change directory to $MYPATH."
+        local -a G=(git -C "$MYPATH")
-            return 3
+        "${G[@]}" rev-parse --is-inside-work-tree >/dev/null 2>&1 || {
-        }
-        git rev-parse --is-inside-work-tree >/dev/null 2>&1 || {
             disp E "Install directory is not a valid Git working tree."
-            popd >/dev/null || return 1
             return 3
         }
 
-        # Resolve the target branch: explicit -b flag > config default > master
-        local target_branch="${branch:-${UPDT_DEFAULT_BRANCH:-master}}"
         branch="$target_branch"
 
         # Detect current local branch and warn+switch when it differs
         local current_branch
-        current_branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)
+        current_branch=$("${G[@]}" rev-parse --abbrev-ref HEAD 2>/dev/null)
         if [[ -n "$current_branch" && "$current_branch" != "$branch" ]]; then
             disp W "Branch mismatch: currently on '$current_branch', configured branch is '$branch'."
             if (( dry_run )); then
@@ -283,21 +342,20 @@ profile_upgrade()
                 disp I "[dry-run] git checkout $branch"
                 disp W "[dry-run] Branch would be changed from '$current_branch' to '$branch'."
             else
-                git fetch origin "$branch" || {
+                "${G[@]}" fetch origin "$branch" || {
                     disp E "Git fetch failed for branch $branch."
-                    popd >/dev/null || return 1
                     return 2
                 }
-                git checkout "$branch" || {
+                "${G[@]}" checkout "$branch" || {
                     disp E "Git checkout failed for branch $branch."
-                    popd >/dev/null || return 1
                     return 2
                 }
                 disp W "Branch has been changed from '$current_branch' to '$branch'."
             fi
         fi
 
-        if ! git diff --quiet || ! git diff --cached --quiet || [[ -n $(git ls-files --others --exclude-standard) ]]; then
+        # status --porcelain: staged, unstaged and untracked (not ignored) files
+        if [[ -n $("${G[@]}" status --porcelain 2>/dev/null) ]]; then
             if (( force_git )); then
                 disp W "Force mode: local Git changes and untracked files will be lost."
                 if (( dry_run )); then
@@ -305,19 +363,16 @@ profile_upgrade()
                     disp I "[dry-run] git reset --hard HEAD"
                     disp I "[dry-run] git clean -fd"
                 else
-                    git fetch --all --prune || {
+                    "${G[@]}" fetch --all --prune || {
                         disp E "Git fetch failed, upgrade not applied."
-                        popd >/dev/null || return 1
                         return 4
                     }
-                    git reset --hard HEAD || {
+                    "${G[@]}" reset --hard HEAD || {
                         disp E "Git reset failed, upgrade not applied."
-                        popd >/dev/null || return 1
                         return 4
                     }
-                    git clean -fd || {
+                    "${G[@]}" clean -fd || {
                         disp E "Git clean failed, upgrade not applied."
-                        popd >/dev/null || return 1
                         return 4
                     }
                 fi
@@ -328,16 +383,16 @@ profile_upgrade()
             fi
         fi
         if (( dry_run )); then
-            disp I "[dry-run] git pull origin $branch"
+            disp I "[dry-run] git pull --ff-only origin $branch"
         else
-            git pull origin "$branch" || {
+            # --ff-only: never create a merge commit nor leave conflict markers
-                disp E "Git pull failed, upgrade not applied."
+            # in the installed scripts; local commits that diverge are reported
-                popd >/dev/null || return 1
+            "${G[@]}" pull --ff-only origin "$branch" || {
+                disp E "Git pull failed, upgrade not applied (see the git message above: local changes in the way, or local commits that diverge from origin/$branch and must be rebased or dropped by hand)."
                 return 2
             }
-            disp I "Successfully upgraded using git."
+            disp I "Successfully upgraded using git. Please log out and log in again."
         fi
-        popd >/dev/null || return 1
     else
         if (( use_archive )); then
             [[ -r "$archive_file" ]] || {
@@ -384,7 +439,7 @@ profile_upgrade()
             if (( dry_run )); then
                 disp I "[dry-run] dwl \"$ARCH_URL\" \"$archive\""
             else
-                dwl "$ARCH_URL" "$archive" || {
+                dwl -t 300 "$ARCH_URL" "$archive" || {
                     disp E "Failed to download archive."
                     rm -rf "$tmpdir"
                     return 6
@@ -393,10 +448,25 @@ profile_upgrade()
         fi
 
         if (( dry_run )); then
-            disp I "[dry-run] tar -xzf \"$archive\" -C \"$tmpdir\""
+            disp I "[dry-run] tar -xf \"$archive\" -C \"$tmpdir\""
             disp I "[dry-run] cp -a <extracted_profile>/. \"$MYPATH\"/"
         else
-            tar -xzf "$archive" -C "$tmpdir" || {
+            # An archive is refused if one of its members is an absolute path or
+            # climbs out of the extraction directory with "..".
+            if ! tar -tf "$archive" >"$tmpdir/list" 2>/dev/null; then
+                disp E "Archive extraction failed (unreadable archive)."
+                rm -rf "$tmpdir"
+                return 7
+            fi
+            if grep -qE '(^/|(^|/)\.\.(/|$))' "$tmpdir/list"; then
+                disp E "Unsafe archive (absolute or '..' path); upgrade refused."
+                rm -rf "$tmpdir"
+                return 7
+            fi
+            rm -f "$tmpdir/list"
+
+            # --no-same-owner: the files belong to the user running the upgrade
+            tar --no-same-owner -xf "$archive" -C "$tmpdir" || {
                 disp E "Archive extraction failed."
                 rm -rf "$tmpdir"
                 return 7
@@ -409,6 +479,16 @@ profile_upgrade()
                 return 8
             fi
 
+            # cp only adds and replaces: a module removed from the new version
+            # stays in profile.d and keeps being loaded. They are listed after
+            # the copy (never deleted: they may be the user's own modules).
+            local f
+            local -a stale=()
+            for f in "$MYPATH"/profile.d/*.sh "$MYPATH"/profile.d/bash-completion/*.sh; do
+                [[ -e "$f" ]] || continue
+                [[ -e "$extracted_root/${f#"$MYPATH"/}" ]] || stale+=("${f#"$MYPATH"/}")
+            done
+
             disp I "Installing new version..."
             cp -a "$extracted_root"/. "$MYPATH"/ || {
                 disp E "Failed to copy new files into $MYPATH."
@@ -416,10 +496,14 @@ profile_upgrade()
                 return 9
             }
 
+            if (( ${#stale[@]} )); then
+                disp W "These files are not part of the new version and are still loaded; remove them if they are not yours: ${stale[*]}"
+            fi
             disp I "Upgrade complete. Please log out and log in again."
             rm -rf "$tmpdir"
         fi
     fi
+    return 0
 }
 export -f profile_upgrade
 # ------------------------------------------------------------------------------