Fix critical and high bugs B1-B6
- B1: Replace recursive get_timezone_mount/get_gpu_devices with literal YAML strings - B3: Expand /home/*/hops glob via compgen -G instead of storing as array literal; fix eval echo ~$SUDO_USER -> getent passwd in uninstall - B4: Correct services source path in setup_firewall (hops_service_definitions.sh -> services) - B5: Replace all ((x++)) with x=$((x + 1)) to avoid set -e abort on zero pre-increment - B6: Add Linux-only guard at top of hops entry point Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -17,12 +17,10 @@ Generated by codebase audit (2026-06-10). Ranked by severity.
|
|||||||
|
|
||||||
## CRITICAL BUGS (breaks primary use cases)
|
## CRITICAL BUGS (breaks primary use cases)
|
||||||
|
|
||||||
### B1 -- Infinite recursion in `services` on Linux [CRITICAL]
|
### B1 -- Infinite recursion in `services` on Linux [CRITICAL] -- RESOLVED
|
||||||
- File: `services:25-46`
|
- File: `services:25-46`
|
||||||
- `get_timezone_mount()` and `get_gpu_devices()` call themselves on the non-Darwin
|
- `get_timezone_mount()` and `get_gpu_devices()` called themselves on the non-Darwin
|
||||||
branch via `echo "$(get_timezone_mount)"`. Hits bash FUNCNEST limit on every
|
branch. Fixed: both functions now return literal YAML strings directly.
|
||||||
Linux compose generation. Main `./hops` install is broken on Linux.
|
|
||||||
- Fix: replace the recursive calls with the literal YAML strings they should emit.
|
|
||||||
|
|
||||||
### B2 -- Brace mismatch in `lib/privileges.sh` [CRITICAL] -- RESOLVED: delete file
|
### B2 -- Brace mismatch in `lib/privileges.sh` [CRITICAL] -- RESOLVED: delete file
|
||||||
- File: `lib/privileges.sh:429,612`
|
- File: `lib/privileges.sh:429,612`
|
||||||
@@ -32,32 +30,22 @@ Generated by codebase audit (2026-06-10). Ranked by severity.
|
|||||||
|
|
||||||
## HIGH BUGS
|
## HIGH BUGS
|
||||||
|
|
||||||
### B3 -- Glob stored as string, directory detection always fails [HIGH]
|
### B3 -- Glob stored as string, directory detection always fails [HIGH] -- RESOLVED
|
||||||
- Files: `hops:154-166`, `uninstall:127-147`
|
- Files: `hops:154-166`, `uninstall:127-147`
|
||||||
- `homelab_dirs=( "/home/*/hops" )` stores a literal glob; the quoted for-loop
|
- Glob removed from array; expanded separately via `compgen -G "/home/*/hops"`.
|
||||||
never expands it. Multi-user detection is broken, `cd "$HOMELAB_DIR"` fails
|
Also fixed `eval echo "~$SUDO_USER"` -> `getent passwd` in `uninstall`.
|
||||||
under `set -e`.
|
|
||||||
- Fix: iterate unquoted or use `compgen -G "/home/*/hops"`.
|
|
||||||
|
|
||||||
### B4 -- Missing service definitions file reference [HIGH]
|
### B4 -- Missing service definitions file reference [HIGH] -- RESOLVED
|
||||||
- File: `install:916`
|
- File: `install:916`
|
||||||
- `setup_firewall()` sources `"$SCRIPT_DIR/hops_service_definitions.sh"` which
|
- Corrected source path from `hops_service_definitions.sh` to `services`.
|
||||||
does not exist (the file is named `services`). Per-service firewall rules are
|
|
||||||
silently never applied.
|
|
||||||
- Fix: correct the filename to `services`.
|
|
||||||
|
|
||||||
### B5 -- `((x++))` aborts script under `set -e` [HIGH]
|
### B5 -- `((x++))` aborts script under `set -e` [HIGH] -- RESOLVED
|
||||||
- Files: `hops:299,317`, `install:784`, and others
|
- Files: `hops`, `install`
|
||||||
- `((running_count++))` returns exit code 1 when the pre-increment value is 0,
|
- All `((x++))` occurrences replaced with `x=$((x + 1))`.
|
||||||
which kills the script under `set -e`.
|
|
||||||
- Fix: use `running_count=$((running_count + 1))` or append `|| true`.
|
|
||||||
|
|
||||||
### B6 -- `hops` entry point is Linux-only despite macOS library support [HIGH]
|
### B6 -- `hops` entry point is Linux-only despite macOS library support [HIGH] -- RESOLVED
|
||||||
- File: `hops:108-136,263`
|
- File: `hops`
|
||||||
- `check_dependencies` requires `systemctl`, `check_system_requirements` calls
|
- Added Linux-only guard at top of script; exits immediately with a clear error on non-Linux.
|
||||||
`free` and `df -BG`, `show_service_status` calls `systemctl`. All Linux-only.
|
|
||||||
The documented entry point fails immediately on macOS.
|
|
||||||
- Fix: add OS guards or document `hops` as Linux-only.
|
|
||||||
|
|
||||||
### B7 -- Port collisions not detected within a selection [HIGH]
|
### B7 -- Port collisions not detected within a selection [HIGH]
|
||||||
- File: `services` (port map)
|
- File: `services` (port map)
|
||||||
|
|||||||
@@ -7,6 +7,11 @@
|
|||||||
# Exit on any error
|
# Exit on any error
|
||||||
set -e
|
set -e
|
||||||
|
|
||||||
|
if [[ "$(uname -s)" != "Linux" ]]; then
|
||||||
|
echo "ERROR: HOPS requires Linux (Ubuntu 20.04+, Debian 11+, or Linux Mint 20+)." >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
# Script version and metadata
|
# Script version and metadata
|
||||||
readonly SCRIPT_VERSION="1.0.0"
|
readonly SCRIPT_VERSION="1.0.0"
|
||||||
readonly SCRIPT_NAME="HOPS"
|
readonly SCRIPT_NAME="HOPS"
|
||||||
@@ -151,10 +156,10 @@ get_installation_status() {
|
|||||||
local status="not_installed"
|
local status="not_installed"
|
||||||
local homelab_dirs=(
|
local homelab_dirs=(
|
||||||
"$HOME/hops"
|
"$HOME/hops"
|
||||||
"/home/*/hops"
|
|
||||||
"/opt/hops"
|
"/opt/hops"
|
||||||
"/srv/hops"
|
"/srv/hops"
|
||||||
)
|
)
|
||||||
|
while IFS= read -r d; do homelab_dirs+=("$d"); done < <(compgen -G "/home/*/hops" 2>/dev/null)
|
||||||
|
|
||||||
# Check for existing installation
|
# Check for existing installation
|
||||||
for dir in "${homelab_dirs[@]}"; do
|
for dir in "${homelab_dirs[@]}"; do
|
||||||
@@ -291,14 +296,14 @@ show_service_status() {
|
|||||||
local service_port="${service_info#*:}"
|
local service_port="${service_info#*:}"
|
||||||
|
|
||||||
if docker ps --format "{{.Names}}" | grep -q "^${service_name}$"; then
|
if docker ps --format "{{.Names}}" | grep -q "^${service_name}$"; then
|
||||||
((total_count++))
|
total_count=$((total_count + 1))
|
||||||
local status_symbol="${GREEN}●${NC}"
|
local status_symbol="${GREEN}●${NC}"
|
||||||
local status_text="Running"
|
local status_text="Running"
|
||||||
|
|
||||||
# Check if port is accessible
|
# Check if port is accessible
|
||||||
if curl -sSf --max-time 2 --connect-timeout 1 "http://localhost:${service_port}" >/dev/null 2>&1; then
|
if curl -sSf --max-time 2 --connect-timeout 1 "http://localhost:${service_port}" >/dev/null 2>&1; then
|
||||||
status_text="Running & Accessible"
|
status_text="Running & Accessible"
|
||||||
((running_count++))
|
running_count=$((running_count + 1))
|
||||||
else
|
else
|
||||||
status_text="Running (starting up)"
|
status_text="Running (starting up)"
|
||||||
status_symbol="${YELLOW}●${NC}"
|
status_symbol="${YELLOW}●${NC}"
|
||||||
@@ -306,7 +311,7 @@ show_service_status() {
|
|||||||
|
|
||||||
printf " %s %-20s %s (:%s)\n" "$status_symbol" "$service_name" "$status_text" "$service_port"
|
printf " %s %-20s %s (:%s)\n" "$status_symbol" "$service_name" "$status_text" "$service_port"
|
||||||
elif docker ps -a --format "{{.Names}}" | grep -q "^${service_name}$"; then
|
elif docker ps -a --format "{{.Names}}" | grep -q "^${service_name}$"; then
|
||||||
((total_count++))
|
total_count=$((total_count + 1))
|
||||||
printf " %s %-20s %s\n" "${RED}●${NC}" "$service_name" "Stopped"
|
printf " %s %-20s %s\n" "${RED}●${NC}" "$service_name" "Stopped"
|
||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
@@ -471,7 +476,7 @@ show_access_info() {
|
|||||||
if docker ps --format "{{.Names}}" | grep -qi "${service_name,,}"; then
|
if docker ps --format "{{.Names}}" | grep -qi "${service_name,,}"; then
|
||||||
local url="http://${local_ip}:${service_port}${service_path}"
|
local url="http://${local_ip}:${service_port}${service_path}"
|
||||||
printf " ${GREEN}●${NC} %-15s %s\n" "$service_name" "$url"
|
printf " ${GREEN}●${NC} %-15s %s\n" "$service_name" "$url"
|
||||||
((active_services++))
|
active_services=$((active_services + 1))
|
||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
|
|
||||||
@@ -518,7 +523,7 @@ show_logs() {
|
|||||||
local date=$(stat -c %y "$log_file" | cut -d' ' -f1)
|
local date=$(stat -c %y "$log_file" | cut -d' ' -f1)
|
||||||
|
|
||||||
printf " %d) %-40s (%s, %s)\n" "$count" "$basename_log" "$size" "$date"
|
printf " %d) %-40s (%s, %s)\n" "$count" "$basename_log" "$size" "$date"
|
||||||
((count++))
|
count=$((count + 1))
|
||||||
done
|
done
|
||||||
|
|
||||||
echo -e "\n${WHITE}Select a log file to view [1-${#log_files[@]}] or 0 to go back: ${NC}"
|
echo -e "\n${WHITE}Select a log file to view [1-${#log_files[@]}] or 0 to go back: ${NC}"
|
||||||
|
|||||||
@@ -333,7 +333,7 @@ EOF
|
|||||||
return 0
|
return 0
|
||||||
fi
|
fi
|
||||||
|
|
||||||
((attempt++))
|
attempt=$((attempt + 1))
|
||||||
done
|
done
|
||||||
|
|
||||||
# Fallback: construct a guaranteed compliant password
|
# Fallback: construct a guaranteed compliant password
|
||||||
@@ -913,8 +913,8 @@ EOF
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
# Allow service ports based on selection
|
# Allow service ports based on selection
|
||||||
if [[ -f "$SCRIPT_DIR/hops_service_definitions.sh" ]]; then
|
if [[ -f "$SCRIPT_DIR/services" ]]; then
|
||||||
source "$SCRIPT_DIR/hops_service_definitions.sh"
|
source "$SCRIPT_DIR/services"
|
||||||
|
|
||||||
for svc in "${SERVICES[@]}"; do
|
for svc in "${SERVICES[@]}"; do
|
||||||
local ports=$(get_service_ports "$svc")
|
local ports=$(get_service_ports "$svc")
|
||||||
@@ -1015,7 +1015,7 @@ EOF
|
|||||||
local main_port=$(echo $ports | cut -d' ' -f1)
|
local main_port=$(echo $ports | cut -d' ' -f1)
|
||||||
if [[ -n "$main_port" ]]; then
|
if [[ -n "$main_port" ]]; then
|
||||||
echo " • $svc: http://$(get_primary_ip):$main_port"
|
echo " • $svc: http://$(get_primary_ip):$main_port"
|
||||||
((service_count++))
|
service_count=$((service_count + 1))
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
|
|||||||
@@ -24,24 +24,18 @@ EOF
|
|||||||
# Get timezone mount path for current platform
|
# Get timezone mount path for current platform
|
||||||
get_timezone_mount() {
|
get_timezone_mount() {
|
||||||
if [[ "$(uname -s)" == "Darwin" ]]; then
|
if [[ "$(uname -s)" == "Darwin" ]]; then
|
||||||
# macOS doesn't need timezone mount, use TZ environment variable
|
|
||||||
echo ""
|
echo ""
|
||||||
else
|
else
|
||||||
# Linux timezone mount
|
echo " - /etc/localtime:/etc/localtime:ro"
|
||||||
echo "$(get_timezone_mount)"
|
|
||||||
fi
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
# Get GPU device access for current platform
|
# Get GPU device access for current platform
|
||||||
get_gpu_devices() {
|
get_gpu_devices() {
|
||||||
if [[ "$(uname -s)" == "Darwin" ]]; then
|
if [[ "$(uname -s)" == "Darwin" ]]; then
|
||||||
# macOS doesn't support GPU passthrough to Docker containers
|
|
||||||
echo ""
|
echo ""
|
||||||
else
|
else
|
||||||
# Linux GPU device access
|
printf " devices:\n - /dev/dri:/dev/dri\n"
|
||||||
cat <<EOF
|
|
||||||
$(get_gpu_devices)
|
|
||||||
EOF
|
|
||||||
fi
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -126,14 +126,14 @@ EOF
|
|||||||
find_homelab_directory() {
|
find_homelab_directory() {
|
||||||
local POSSIBLE_DIRS=(
|
local POSSIBLE_DIRS=(
|
||||||
"$HOME/hops"
|
"$HOME/hops"
|
||||||
"/home/*/hops"
|
|
||||||
"/opt/hops"
|
"/opt/hops"
|
||||||
"/srv/hops"
|
"/srv/hops"
|
||||||
)
|
)
|
||||||
|
while IFS= read -r d; do POSSIBLE_DIRS+=("$d"); done < <(compgen -G "/home/*/hops" 2>/dev/null)
|
||||||
|
|
||||||
# Try to find from running user's home first
|
|
||||||
if [[ -n "$SUDO_USER" ]]; then
|
if [[ -n "$SUDO_USER" ]]; then
|
||||||
local user_home=$(eval echo "~$SUDO_USER")
|
local user_home
|
||||||
|
user_home=$(getent passwd "$SUDO_USER" | cut -d: -f6)
|
||||||
POSSIBLE_DIRS=("$user_home/hops" "${POSSIBLE_DIRS[@]}")
|
POSSIBLE_DIRS=("$user_home/hops" "${POSSIBLE_DIRS[@]}")
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user