From 3eb618923cf110d6fa97b90958fee8c372d4abdb Mon Sep 17 00:00:00 2001 From: Stefan Strobl Date: Thu, 25 Dec 2025 11:21:50 +0100 Subject: [PATCH] refactor: improve error handling and fix critical bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhance code robustness and fix several critical issues identified during code review: **Error Handling:** - Add set -euo pipefail to all modules for consistent error detection - Add sleep delays after partition refresh to prevent race conditions **Bug Fixes:** - Fix package verification flag from -R to -r (xbps-query) - Fix swap file creation for btrfs (use dd instead of fallocate) - Add atomic fstab updates with backup mechanism **Optimizations:** - Remove @swap btrfs subvolume, use regular directory instead - Direct dd usage for btrfs swap files (COW-compatible) - Optimize swap creation logic per filesystem type **Security:** - Add GRUB/LUKS2 compatibility check with version warning - Create fstab backup before modifications Changes affect 14 files across all installer phases. All changes improve reliability, error detection, and filesystem compatibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- README.md | 3 ++- src/config.sh | 1 + src/encryption.sh | 1 + src/filesystems.sh | 2 +- src/locale.sh | 1 + src/logging.sh | 1 + src/mounts.sh | 1 + src/packages.sh | 3 ++- src/partitioning.sh | 5 ++++ src/postinstall.sh | 64 +++++++++++++++++++++++++++++++++------------ src/rollback.sh | 1 + src/sanity.sh | 1 + src/services.sh | 1 + src/users.sh | 1 + 14 files changed, 67 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index d800c5c..d265cfe 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,8 @@ Bei Auswahl von btrfs werden automatisch folgende Subvolumes angelegt: - `@var` → `/var` (Variable Daten) - `@log` → `/var/log` (System-Logs) - `@snapshots` → `/.snapshots` (Snapshot-Speicher) -- `@swap` → `/swap` (Swap-File Container) + +**Hinweis:** Das Swap-File wird direkt im Root-Subvolume unter `/swap/swapfile` erstellt (kein separates Subvolume), da dies bei btrfs performanter ist. ## Package-Liste diff --git a/src/config.sh b/src/config.sh index ea804b7..56abcd1 100644 --- a/src/config.sh +++ b/src/config.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Centralize user-provided inputs to avoid scattered magic values. diff --git a/src/encryption.sh b/src/encryption.sh index c46e85b..2b78453 100644 --- a/src/encryption.sh +++ b/src/encryption.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Protect user data with full-disk encryption while keeping boot reliable. diff --git a/src/filesystems.sh b/src/filesystems.sh index 47606b6..8d1975f 100644 --- a/src/filesystems.sh +++ b/src/filesystems.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Provide a stable filesystem base aligned with user intent. @@ -83,7 +84,6 @@ format_filesystems() { btrfs subvolume create "$temp_mount/@var" btrfs subvolume create "$temp_mount/@log" btrfs subvolume create "$temp_mount/@snapshots" - btrfs subvolume create "$temp_mount/@swap" umount "$temp_mount" rmdir "$temp_mount" trap - EXIT diff --git a/src/locale.sh b/src/locale.sh index 5da8a18..e01bde5 100644 --- a/src/locale.sh +++ b/src/locale.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Configure system locale, timezone, and keyboard layout for international users. diff --git a/src/logging.sh b/src/logging.sh index 0f6cc72..1ac805e 100644 --- a/src/logging.sh +++ b/src/logging.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Provide transparency and auditability during destructive operations. diff --git a/src/mounts.sh b/src/mounts.sh index bb2146c..38306e6 100644 --- a/src/mounts.sh +++ b/src/mounts.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Present a clean mount tree to the installer. diff --git a/src/packages.sh b/src/packages.sh index 912db58..21d9497 100644 --- a/src/packages.sh +++ b/src/packages.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Install all required packages for a bootable Void Linux system. @@ -88,7 +89,7 @@ packages_install() { log_info "Verifying package installation..." local critical_packages=("base-system" "linux" "grub" "cryptsetup") for pkg in "${critical_packages[@]}"; do - if ! xbps-query -R "$MOUNT_ROOT" "$pkg" >/dev/null 2>&1; then + if ! xbps-query -r "$MOUNT_ROOT" "$pkg" >/dev/null 2>&1; then die "Critical package not found: $pkg" fi done diff --git a/src/partitioning.sh b/src/partitioning.sh index 9af1302..50d08ad 100644 --- a/src/partitioning.sh +++ b/src/partitioning.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Establish a predictable disk layout that supports encrypted root. @@ -53,12 +54,16 @@ refresh_partition_table() { if command -v partprobe >/dev/null 2>&1; then partprobe "$disk" + sleep 1 elif command -v partx >/dev/null 2>&1; then partx -u "$disk" + sleep 1 elif command -v udevadm >/dev/null 2>&1; then udevadm settle + sleep 1 else log_warn "No partition refresh tool available; proceeding without refresh." + sleep 2 fi } diff --git a/src/postinstall.sh b/src/postinstall.sh index eb0f574..b800433 100644 --- a/src/postinstall.sh +++ b/src/postinstall.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Ensure the system can boot with encrypted root after installation. @@ -121,35 +122,50 @@ update_fstab_entry() { fi local line="UUID=$uuid $mount_point $fs_type $options 0 0" + # Create backup before first modification + if [[ -f /etc/fstab ]] && [[ ! -f /etc/fstab.backup ]]; then + cp /etc/fstab /etc/fstab.backup + log_info "Created fstab backup at /etc/fstab.backup" + fi + if awk -v mp="$mount_point" '$2==mp {found=1} END {exit found?0:1}' /etc/fstab; then + # Update existing entry awk -v mp="$mount_point" -v line="$line" 'BEGIN{OFS=" "} $2==mp {$0=line} {print}' /etc/fstab > /etc/fstab.tmp + # Atomic move with sync + sync /etc/fstab.tmp mv /etc/fstab.tmp /etc/fstab else + # Append new entry printf '%s\n' "$line" >> /etc/fstab fi } create_swapfile() { local swap_size="$1" + local size_num + local size_unit + local bs="1M" + local count - # Try fallocate first, fall back to dd if it fails - if ! fallocate -l "$swap_size" /swap/swapfile 2>/dev/null; then - echo "fallocate failed, using dd as fallback..." - local size_num - local size_unit - local bs="1M" - local count + size_num="${swap_size%%[^0-9.]*}" + size_unit="${swap_size##*[0-9.]}" - size_num="${swap_size%%[^0-9.]*}" - size_unit="${swap_size##*[0-9.]}" - - case "$size_unit" in - GiB|G|GB) count=$(awk "BEGIN {print int($size_num * 1024)}") ;; - MiB|M|MB|"") count=$(awk "BEGIN {print int($size_num)}") ;; - *) echo "Warning: unknown size unit, assuming MiB"; count=$(awk "BEGIN {print int($size_num)}") ;; - esac + case "$size_unit" in + GiB|G|GB) count=$(awk "BEGIN {print int($size_num * 1024)}") ;; + MiB|M|MB|"") count=$(awk "BEGIN {print int($size_num)}") ;; + *) echo "Warning: unknown size unit, assuming MiB"; count=$(awk "BEGIN {print int($size_num)}") ;; + esac + # Use dd for btrfs, fallocate for ext4 + if [[ "$FS_TYPE" == "btrfs" ]]; then + log_info "Creating swap file with dd (btrfs requires this)..." dd if=/dev/zero of=/swap/swapfile bs="$bs" count="$count" status=progress + else + log_info "Creating swap file with fallocate..." + if ! fallocate -l "$swap_size" /swap/swapfile 2>/dev/null; then + log_warn "fallocate failed, falling back to dd..." + dd if=/dev/zero of=/swap/swapfile bs="$bs" count="$count" status=progress + fi fi } @@ -162,7 +178,6 @@ if [[ "$FS_TYPE" == "btrfs" ]]; then update_fstab_entry /var "btrfs" "defaults,subvol=@var" "$ROOT_UUID" update_fstab_entry /var/log "btrfs" "defaults,subvol=@log" "$ROOT_UUID" update_fstab_entry /.snapshots "btrfs" "defaults,subvol=@snapshots" "$ROOT_UUID" - update_fstab_entry /swap "btrfs" "defaults,subvol=@swap" "$ROOT_UUID" else update_fstab_entry / "ext4" "defaults" "$ROOT_UUID" fi @@ -187,6 +202,23 @@ if [[ "$SWAP_SIZE" != "0" ]]; then fi fi +# Check GRUB version for LUKS2 compatibility +if [[ "${LUKS_VERSION:-2}" == "2" ]]; then + log_info "Checking GRUB version for LUKS2 compatibility..." + if command -v grub-install >/dev/null 2>&1; then + grub_version=$(grub-install --version 2>/dev/null | grep -oP '\d+\.\d+' | head -n1) + if [[ -n "$grub_version" ]]; then + # Compare version (2.06 is minimum for reliable LUKS2 support) + if awk "BEGIN {exit !($grub_version < 2.06)}"; then + log_warn "GRUB version $grub_version detected. LUKS2 requires GRUB >= 2.06 for reliable boot support." + log_warn "Consider using LUKS version 1 if you experience boot issues." + else + log_info "GRUB version $grub_version - LUKS2 support OK" + fi + fi + fi +fi + # Ensure GRUB unlocks the encrypted root and passes the LUKS UUID to initramfs. mkdir -p /etc/default touch /etc/default/grub diff --git a/src/rollback.sh b/src/rollback.sh index 5171c17..d780235 100644 --- a/src/rollback.sh +++ b/src/rollback.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Reduce fallout if a phase fails mid-way. diff --git a/src/sanity.sh b/src/sanity.sh index 6f422bd..f16ec7e 100644 --- a/src/sanity.sh +++ b/src/sanity.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Prevent catastrophic mistakes before any disk operations. diff --git a/src/services.sh b/src/services.sh index ee9d627..5449d57 100644 --- a/src/services.sh +++ b/src/services.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Enable essential system services for network connectivity and basic functionality. diff --git a/src/users.sh b/src/users.sh index 6e8b4ec..f9eca87 100644 --- a/src/users.sh +++ b/src/users.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +set -euo pipefail # === Motivation === # Create user accounts with proper permissions for system access.