Fix: address code review findings across core, tui, and tab scripts (#1296)

* fix: address code review findings across core, tui, and tab scripts

* chore(docs): regenerate walkthrough.md

core/src/inner.rs

@@ -140,7 +140,9 @@ impl Entry {
                         SystemDataType::CommandExists => values
                             .iter()
                             .all(|command| which::which(command).is_ok() == *matches),
-                        SystemDataType::FileExists => values.iter().all(|p| Path::new(p).is_file()),
+                        SystemDataType::FileExists => {
+                            values.iter().all(|p| Path::new(p).is_file() == *matches)
+                        }
                     }
                 },
             )
@@ -246,7 +248,10 @@ fn get_shebang(script_path: &Path, validate: bool) -> Option<(String, Vec<String
         return default_executable();
     }
 
-    let first_line = reader.lines().next().unwrap().unwrap();
+    let first_line = match reader.lines().next() {
+        Some(Ok(line)) => line,
+        _ => return default_executable(),
+    };
 
     let mut parts = first_line.split_whitespace();
 

core/tabs/applications-setup/communication-apps/discord-setup.sh

@@ -7,9 +7,12 @@ installDiscord() {
         printf "%b\n" "${YELLOW}Installing Discord...${RC}"
         case "$PACKAGER" in
             apt-get|nala)
-                curl -Lo discord.deb "https://discord.com/api/download?platform=linux&format=deb"
+                # Ensure the downloaded .deb is removed even on failure under `set -e`.
+                trap 'rm -f discord.deb' EXIT
+                curl -fLo discord.deb "https://discord.com/api/download?platform=linux&format=deb"
                 "$ESCALATION_TOOL" "$PACKAGER" install -y ./discord.deb
-                "$ESCALATION_TOOL" rm discord.deb
+                rm -f discord.deb
+                trap - EXIT
                 ;;
             zypper|eopkg)
                 "$ESCALATION_TOOL" "$PACKAGER" install -y discord

core/tabs/applications-setup/linutil-installer.sh

@@ -5,10 +5,12 @@
 install_extra() {
     printf "%b\n" "${YELLOW}Installing the manpage...${RC}"
     "$ESCALATION_TOOL" mkdir -p /usr/share/man/man1
-    curl 'https://raw.githubusercontent.com/ChrisTitusTech/linutil/refs/heads/main/man/linutil.1' | "$ESCALATION_TOOL" tee '/usr/share/man/man1/linutil.1' > /dev/null
+    # -f makes curl exit non-zero on HTTP errors so a 404/5xx body is not written
+    # as the manpage; -sSL keeps the curl quiet but shows the error if it fails.
+    curl -fsSL 'https://raw.githubusercontent.com/ChrisTitusTech/linutil/refs/heads/main/man/linutil.1' | "$ESCALATION_TOOL" tee '/usr/share/man/man1/linutil.1' > /dev/null
     printf "%b\n" "${YELLOW}Creating a Desktop Entry...${RC}"
     "$ESCALATION_TOOL" mkdir -p /usr/share/applications
-    curl 'https://raw.githubusercontent.com/ChrisTitusTech/linutil/refs/heads/main/linutil.desktop' | "$ESCALATION_TOOL" tee /usr/share/applications/linutil.desktop > /dev/null
+    curl -fsSL 'https://raw.githubusercontent.com/ChrisTitusTech/linutil/refs/heads/main/linutil.desktop' | "$ESCALATION_TOOL" tee /usr/share/applications/linutil.desktop > /dev/null
     printf "%b\n" "${GREEN}Done.${RC}"
 }
 

core/tabs/common-script.sh

@@ -97,7 +97,11 @@ checkEscalationTool() {
     ## Check for escalation tools.
     if [ -z "$ESCALATION_TOOL_CHECKED" ]; then
         if [ "$(id -u)" = "0" ]; then
-            ESCALATION_TOOL="eval"
+            # `env` runs the given command with the current environment without
+            # re-tokenizing its arguments, unlike `eval` which would re-parse
+            # quoting/globs and enable injection on paths containing spaces or
+            # shell metacharacters.
+            ESCALATION_TOOL="env"
             ESCALATION_TOOL_CHECKED=true
             printf "%b\n" "${CYAN}Running as root, no escalation needed${RC}"
             return 0
@@ -155,16 +159,21 @@ checkPackageManager() {
 checkSuperUser() {
     ## Check SuperUser Group
     SUPERUSERGROUP='wheel sudo root'
+    USER_GROUPS=$(id -nG)
     for sug in ${SUPERUSERGROUP}; do
-        if groups | grep -q "${sug}"; then
-            SUGROUP=${sug}
-            printf "%b\n" "${CYAN}Super user group ${SUGROUP}${RC}"
-            break
-        fi
+        # Match whole-word group names; unanchored grep would match e.g.
+        # `wheelchair`, `rooted`, or `sudoers` as the privileged group.
+        case " ${USER_GROUPS} " in
+            *" ${sug} "*)
+                SUGROUP=${sug}
+                printf "%b\n" "${CYAN}Super user group ${SUGROUP}${RC}"
+                break
+                ;;
+        esac
     done
 
-    ## Check if member of the sudo group.
-    if ! groups | grep -q "${SUGROUP}"; then
+    ## Fail fast if not a member of any super-user group.
+    if [ -z "$SUGROUP" ]; then
         printf "%b\n" "${RED}You need to be a member of the sudo group to run me!${RC}"
         exit 1
     fi

core/tabs/common-service-script.sh

@@ -80,6 +80,7 @@ startAndEnableService() {
             ;;
         sv)
             enableService "$1"
+            startService "$1"
             ;;
     esac
 }

core/tabs/system-setup/gaming-setup.sh

@@ -133,11 +133,11 @@ installDepend() {
         apt-get | nala)
             run_install_step "Enable i386 architecture" "$ESCALATION_TOOL" dpkg --add-architecture i386
             run_install_step "Refresh package indexes" "$ESCALATION_TOOL" "$PACKAGER" update
-            
+
             run_install_step "Install base dependencies" "$ESCALATION_TOOL" "$PACKAGER" install -y $DEPENDENCIES
-            
+
             DISTRO_DEPS="libasound2-plugins:i386 libsdl2-2.0-0:i386 libdbus-1-3:i386 libsqlite3-0:i386 wine32:i386"
-            apt-cache show software-properties-common >/dev/null 2>&1 && DISTRO_DEPS="$DISTRO_DEPS software-properties-common"        
+            apt-cache show software-properties-common >/dev/null 2>&1 && DISTRO_DEPS="$DISTRO_DEPS software-properties-common"
             run_install_step "Install distro-specific dependencies" "$ESCALATION_TOOL" "$PACKAGER" install -y $DISTRO_DEPS
             ;;
         dnf)

core/tabs/utils/ssh.sh

@@ -29,8 +29,6 @@ ask_for_host_details() {
         printf "%b\n" "    HostName $host"
         printf "%b\n" "    User $user"
         printf "%b\n" "    IdentityFile ~/.ssh/id_rsa"
-        printf "%b\n" "    StrictHostKeyChecking no"
-        printf "%b\n" "    UserKnownHostsFile=/dev/null"
     } >> ~/.ssh/config
     printf "%b\n" "Host $host_alias added successfully."
 }

tui/src/confirmation.rs

@@ -45,7 +45,7 @@ impl ConfirmPrompt {
     }
 
     pub fn scroll_down(&mut self) {
-        if self.scroll + self.inner_area_height < self.names.len() - 1 {
+        if self.scroll + self.inner_area_height < self.names.len().saturating_sub(1) {
             self.scroll += 1;
         }
     }

tui/src/floating_text.rs

@@ -52,13 +52,11 @@ impl<'a> FloatingText<'a> {
 
     pub fn from_command(command: &Command, title: &str, wrap_words: bool) -> Self {
         let src = match command {
-            Command::Raw(cmd) => Some(cmd.clone()),
+            Command::Raw(cmd) => cmd.clone(),
             Command::LocalFile { file, .. } => std::fs::read_to_string(file)
-                .map_err(|_| format!("File not found: {file:?}"))
-                .ok(),
-            Command::None => None,
-        }
-        .unwrap();
+                .unwrap_or_else(|e| format!("<preview unavailable for {}: {e}>", file.display())),
+            Command::None => String::from("<no preview available>"),
+        };
 
         let processed_text = Self::get_highlighted_string(&src).unwrap_or_else(|| Text::from(src));
 

tui/src/running_command.rs

@@ -304,15 +304,15 @@ impl RunningCommand {
     }
 
     fn screen(&mut self, size: Size) -> Screen {
-        // Resize the emulated pty
-        self.pty_master
-            .resize(PtySize {
-                rows: size.height,
-                cols: size.width,
-                pixel_width: 0,
-                pixel_height: 0,
-            })
-            .unwrap();
+        // Resize the emulated pty; ignore failures (e.g., pty already closed
+        // after the child exited, or zero-dimension target on extreme resizes)
+        // since the screen still renders from the existing buffer.
+        let _ = self.pty_master.resize(PtySize {
+            rows: size.height,
+            cols: size.width,
+            pixel_width: 0,
+            pixel_height: 0,
+        });
 
         // Process the buffer with a parser with the current screen size
         // We don't actually need to create a new parser every time, but it is so much easier this
@@ -340,9 +340,17 @@ impl RunningCommand {
 
     /// Send SIGHUB signal, *not* SIGKILL or SIGTERM, to the child process
     pub fn kill_child(&mut self) {
-        if !self.is_finished() {
-            let mut killer = self.child_killer.take().unwrap().recv().unwrap();
-            killer.kill().unwrap();
+        if self.is_finished() {
+            return;
+        }
+        let Some(rx) = self.child_killer.take() else {
+            // Already consumed by a prior kill_child call.
+            return;
+        };
+        // recv can fail if the spawning thread panicked (e.g., spawn_command
+        // failed); in that case the child never existed, so there's nothing to kill.
+        if let Ok(mut killer) = rx.recv() {
+            let _ = killer.kill();
         }
     }