From 32f989653ea911529ee9106227f4504ca9fcfdac Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 12 Jan 2026 21:43:02 +0000
Subject: [PATCH] Address code review feedback: improve error handling and
 VirtualBox detection

Co-authored-by: kuhyx <147418882+kuhyx@users.noreply.github.com>
---
 .../pacman/install_pacman_wrapper.sh          | 18 +++++++++--
 .../pacman/pacman_wrapper.sh                  | 30 +++++++++++++++----
 .../virtualbox/enforce_vbox_hosts.sh          | 10 ++++---
 3 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/scripts/digital_wellbeing/pacman/install_pacman_wrapper.sh b/scripts/digital_wellbeing/pacman/install_pacman_wrapper.sh
index a9601d4..0f3b58f 100755
--- a/scripts/digital_wellbeing/pacman/install_pacman_wrapper.sh
+++ b/scripts/digital_wellbeing/pacman/install_pacman_wrapper.sh
@@ -90,11 +90,23 @@ chmod 755 "$INTEGRITY_DIR"
 # Generate checksums of policy files for integrity verification
 echo -e "${BLUE}Generating integrity checksums for policy files...${NC}"
 {
-  sha256sum "$BLOCKED_DEST" 2>/dev/null || true
-  sha256sum "$GREYLIST_DEST" 2>/dev/null || true
-  sha256sum "$WHITELIST_DEST" 2>/dev/null || true
+  if [[ -f "$BLOCKED_DEST" ]]; then
+    sha256sum "$BLOCKED_DEST" || { echo -e "${RED}Failed to checksum blocked list${NC}"; exit 1; }
+  fi
+  if [[ -f "$GREYLIST_DEST" ]]; then
+    sha256sum "$GREYLIST_DEST" || { echo -e "${RED}Failed to checksum greylist${NC}"; exit 1; }
+  fi
+  if [[ -f "$WHITELIST_DEST" ]]; then
+    sha256sum "$WHITELIST_DEST" || { echo -e "${RED}Failed to checksum whitelist${NC}"; exit 1; }
+  fi
 } > "$INTEGRITY_FILE"
 
+# Verify integrity file was created and has content
+if [[ ! -s "$INTEGRITY_FILE" ]]; then
+  echo -e "${RED}Error: Integrity file was not created or is empty${NC}"
+  exit 1
+fi
+
 # Make integrity file immutable
 chmod 400 "$INTEGRITY_FILE"
 if command -v chattr > /dev/null 2>&1; then
diff --git a/scripts/digital_wellbeing/pacman/pacman_wrapper.sh b/scripts/digital_wellbeing/pacman/pacman_wrapper.sh
index 894b925..69189e9 100755
--- a/scripts/digital_wellbeing/pacman/pacman_wrapper.sh
+++ b/scripts/digital_wellbeing/pacman/pacman_wrapper.sh
@@ -747,12 +747,21 @@ remove_installed_greylisted_packages "$@"
 # If VirtualBox was involved in this operation, enforce hosts file sharing
 enforce_vbox_hosts_if_needed() {
   # Only check after install operations
-  if [[ ${1:-} != "-S"* && ${1:-} != "-U"* ]]; then
+  if [[ -z ${1:-} ]]; then
     return 0
   fi
   
-  # Check if VirtualBox is installed
-  if ! "$PACMAN_BIN" -Qq virtualbox > /dev/null 2>&1; then
+  if [[ $1 != "-S"* && $1 != "-U"* ]]; then
+    return 0
+  fi
+  
+  # Check if ANY VirtualBox package is installed (use broader search)
+  local vbox_installed=0
+  if "$PACMAN_BIN" -Qq 2>/dev/null | grep -Eq '^(virtualbox|vbox)'; then
+    vbox_installed=1
+  fi
+  
+  if [[ $vbox_installed -eq 0 ]]; then
     return 0
   fi
   
@@ -771,6 +780,7 @@ enforce_vbox_hosts_if_needed() {
   fi
   
   if [[ -z $vbox_enforce_script ]]; then
+    echo -e "${YELLOW}VirtualBox detected but enforcement script not found. Hosts file may not be enforced in VMs.${NC}" >&2
     return 0
   fi
   
@@ -779,12 +789,20 @@ enforce_vbox_hosts_if_needed() {
     return 0
   fi
   
-  # VirtualBox is installed but enforcement not applied
+  # VirtualBox is installed but enforcement not applied - this is critical
   echo -e "${YELLOW}VirtualBox detected. Applying /etc/hosts enforcement to VMs...${NC}" >&2
   if [[ $EUID -ne 0 ]]; then
-    sudo bash "$vbox_enforce_script" enforce || echo -e "${RED}Failed to enforce hosts on VirtualBox VMs${NC}" >&2
+    if ! sudo bash "$vbox_enforce_script" enforce; then
+      echo -e "${RED}CRITICAL: Failed to enforce hosts on VirtualBox VMs!${NC}" >&2
+      echo -e "${RED}VMs may bypass /etc/hosts restrictions. Please run manually:${NC}" >&2
+      echo -e "${RED}  sudo $vbox_enforce_script enforce${NC}" >&2
+    fi
   else
-    bash "$vbox_enforce_script" enforce || echo -e "${RED}Failed to enforce hosts on VirtualBox VMs${NC}" >&2
+    if ! bash "$vbox_enforce_script" enforce; then
+      echo -e "${RED}CRITICAL: Failed to enforce hosts on VirtualBox VMs!${NC}" >&2
+      echo -e "${RED}VMs may bypass /etc/hosts restrictions. Please run manually:${NC}" >&2
+      echo -e "${RED}  $vbox_enforce_script enforce${NC}" >&2
+    fi
   fi
 }
 
diff --git a/scripts/digital_wellbeing/virtualbox/enforce_vbox_hosts.sh b/scripts/digital_wellbeing/virtualbox/enforce_vbox_hosts.sh
index a69c13b..6806871 100644
--- a/scripts/digital_wellbeing/virtualbox/enforce_vbox_hosts.sh
+++ b/scripts/digital_wellbeing/virtualbox/enforce_vbox_hosts.sh
@@ -92,14 +92,16 @@ BACKUP_HOSTS_FILE="/etc/hosts.pre-vbox-sync"
 
 # Function to check if running in VirtualBox
 is_virtualbox() {
-  if command -v dmidecode > /dev/null 2>&1; then
-    if sudo dmidecode -s system-product-name 2>/dev/null | grep -qi "VirtualBox"; then
+  # First try systemd-detect-virt (no root required)
+  if command -v systemd-detect-virt > /dev/null 2>&1; then
+    if systemd-detect-virt 2>/dev/null | grep -qi "oracle"; then
       return 0
     fi
   fi
   
-  if command -v systemd-detect-virt > /dev/null 2>&1; then
-    if systemd-detect-virt | grep -qi "oracle"; then
+  # Then try dmidecode (requires root, but script should already be running as root)
+  if command -v dmidecode > /dev/null 2>&1; then
+    if dmidecode -s system-product-name 2>/dev/null | grep -qi "VirtualBox"; then
       return 0
     fi
   fi