Merge pull request #76 from StarFleetCPTN/development

Fixes

entrypoint.sh

@@ -16,15 +16,49 @@ if [ -n "${PUID}" ] && [ -n "${PGID}" ]; then
         echo "Detected Alpine Linux, using busybox usermod/groupmod..."
         
         # Update group ID first
-        if [ "$(getent group ${USERNAME} | cut -d: -f3)" != "${PGID}" ]; then
-            echo "Updating GID to ${PGID}..."
-            groupmod -g ${PGID} ${USERNAME} || echo "⚠️ Failed to change GID"
-        fi
-        
-        # Update user ID
-        if [ "$(id -u ${USERNAME})" != "${PUID}" ]; then
-            echo "Updating UID to ${PUID}..."
-            usermod -u ${PUID} ${USERNAME} || echo "⚠️ Failed to change UID"
+        CURRENT_GID=$(getent group ${USERNAME} | cut -d: -f3)
+        CURRENT_UID=$(id -u ${USERNAME})
+
+        if [ "${CURRENT_GID}" != "${PGID}" ] || [ "${CURRENT_UID}" != "${PUID}" ]; then
+            echo "Attempting to update UID/GID to ${PUID}:${PGID} using delete/recreate..."
+            
+            # Delete existing user and group, ignoring errors
+            deluser ${USERNAME} > /dev/null 2>&1 || true
+            delgroup ${USERNAME} > /dev/null 2>&1 || true
+            
+            # Add group with the specified GID
+            echo "Adding group ${USERNAME} with GID ${PGID}"
+            if ! addgroup -g ${PGID} ${USERNAME}; then
+                echo "⚠️ Failed to add group ${USERNAME} with GID ${PGID}."
+                # Exiting because user creation will likely fail
+                exit 1
+            fi
+            
+            # Add user with the specified UID and GID
+            # Use -G for primary group with adduser in BusyBox
+            # Use -h /app for home directory (consistent with expectations)
+            # Use -s /bin/sh for shell
+            # Use -D for no password (system user)
+            echo "Adding user ${USERNAME} with UID ${PUID}"
+            if ! adduser -u ${PUID} -G ${USERNAME} -h /app -s /bin/sh -D ${USERNAME}; then
+                 echo "⚠️ Failed to add user ${USERNAME} with UID ${PUID} and group ${USERNAME}."
+                 # Exiting because the application cannot run as the correct user
+                 exit 1
+            fi
+            
+            # Verify the change
+            FINAL_UID=$(id -u ${USERNAME} 2>/dev/null || echo "error")
+            FINAL_GID=$(getent group ${USERNAME} | cut -d: -f3 2>/dev/null || echo "error")
+            
+            if [ "${FINAL_UID}" = "${PUID}" ] && [ "${FINAL_GID}" = "${PGID}" ]; then
+                echo "✅ Successfully updated UID/GID to ${PUID}:${PGID}"
+            else
+                echo "⚠️ Verification failed after update. Target: ${PUID}:${PGID}, Actual: ${FINAL_UID}:${FINAL_GID}"
+                # Exiting because the UID/GID is not correct
+                exit 1
+            fi
+        else
+            echo "UID/GID ${PUID}:${PGID} already set."
         fi
     else
         echo "Non-Alpine system, using standard user management..."

internal/db/migrations/011a_recover_transfer_configs_rename.go

@@ -0,0 +1,47 @@
+package migrations
+
+import (
+	"fmt"
+
+	"github.com/go-gormigrate/gormigrate/v2"
+	"gorm.io/gorm"
+)
+
+// RecoverTransferConfigsRename checks for and corrects a specific inconsistent state
+// left by a potentially failed run of migration 012, where the transfer_configs
+// table might have been left renamed as _transfer_configs_old.
+func RecoverTransferConfigsRename() *gormigrate.Migration {
+	return &gormigrate.Migration{
+		ID: "011a_recover_transfer_configs_rename",
+		Migrate: func(tx *gorm.DB) error {
+			fmt.Println("Running migration 011a: Checking for transfer_configs rename recovery...")
+
+			var oldTableExists int
+			tx.Raw("SELECT count(*) FROM sqlite_master WHERE type='table' AND name='_transfer_configs_old'").Scan(&oldTableExists)
+
+			var newTableExists int
+			tx.Raw("SELECT count(*) FROM sqlite_master WHERE type='table' AND name='transfer_configs'").Scan(&newTableExists)
+
+			if oldTableExists > 0 && newTableExists == 0 {
+				fmt.Println("Found _transfer_configs_old table but not transfer_configs. Attempting recovery rename...")
+				if err := tx.Exec("ALTER TABLE _transfer_configs_old RENAME TO transfer_configs").Error; err != nil {
+					return fmt.Errorf("failed to rename _transfer_configs_old back to transfer_configs: %w", err)
+				}
+				fmt.Println("Successfully renamed _transfer_configs_old to transfer_configs.")
+			} else if oldTableExists > 0 && newTableExists > 0 {
+				// This state shouldn't ideally happen if migration 012 followed its logic,
+				// but indicates a potential issue. Maybe drop the old one? For now, just log.
+				fmt.Println("Warning: Both transfer_configs and _transfer_configs_old tables exist. Manual inspection might be needed.")
+			} else {
+				fmt.Println("No recovery needed for transfer_configs rename.")
+			}
+
+			return nil
+		},
+		Rollback: func(tx *gorm.DB) error {
+			// Rollback doesn't make sense for a recovery step.
+			fmt.Println("Rollback for migration 011a_recover_transfer_configs_rename is not applicable.")
+			return nil
+		},
+	}
+}

internal/db/migrations/012_alter_boolean_defaults.go

@@ -178,6 +178,12 @@ func AlterBooleanDefaults() *gormigrate.Migration {
 				fmt.Printf("Recreating table %s...\n", tableName)
 				oldTableName := fmt.Sprintf("_%s_old", tableName)
 
+				// Drop the old temp table if it exists from a previous failed run
+				if err := tx.Exec(fmt.Sprintf("DROP TABLE IF EXISTS %s", oldTableName)).Error; err != nil {
+					// Log the error but proceed, as the rename might still work or fail for the intended reason
+					fmt.Printf("Warning: failed to drop potential leftover table %s: %v\n", oldTableName, err)
+				}
+
 				// Rename old table
 				if err := tx.Exec(fmt.Sprintf("ALTER TABLE %s RENAME TO %s", tableName, oldTableName)).Error; err == nil {
 					fmt.Printf("Renamed %s to %s.\n", tableName, oldTableName)

internal/db/migrations/013_cleanup_invalid_booleans.go

@@ -0,0 +1,91 @@
+package migrations
+
+import (
+	"fmt"
+
+	"github.com/go-gormigrate/gormigrate/v2"
+	"gorm.io/gorm"
+)
+
+// CleanupInvalidBooleans updates boolean columns represented as integers
+// to ensure they only contain valid values (0, 1, or NULL).
+func CleanupInvalidBooleans() *gormigrate.Migration {
+	return &gormigrate.Migration{
+		ID: "013_cleanup_invalid_booleans",
+		Migrate: func(tx *gorm.DB) error {
+			fmt.Println("Running migration 013: Cleaning up invalid boolean values...")
+
+			// Target: transfer_configs.delete_after_transfer
+			// Set any non-NULL value that is not 0 or 1 to 0 (false)
+			sql := `UPDATE transfer_configs
+					SET delete_after_transfer = 0
+					WHERE delete_after_transfer IS NOT NULL AND delete_after_transfer NOT IN (0, 1);`
+
+			if err := tx.Exec(sql).Error; err != nil {
+				return fmt.Errorf("failed to cleanup delete_after_transfer in transfer_configs: %w", err)
+			}
+			fmt.Println("Cleaned up invalid values in transfer_configs.delete_after_transfer.")
+
+			// Target: transfer_configs.archive_enabled
+			sql = `UPDATE transfer_configs
+					SET archive_enabled = 0
+					WHERE archive_enabled IS NOT NULL AND archive_enabled NOT IN (0, 1);`
+			if err := tx.Exec(sql).Error; err != nil {
+				return fmt.Errorf("failed to cleanup archive_enabled in transfer_configs: %w", err)
+			}
+			fmt.Println("Cleaned up invalid values in transfer_configs.archive_enabled.")
+
+			// Target: transfer_configs.skip_processed_files
+			sql = `UPDATE transfer_configs
+					SET skip_processed_files = 0
+					WHERE skip_processed_files IS NOT NULL AND skip_processed_files NOT IN (0, 1);`
+			if err := tx.Exec(sql).Error; err != nil {
+				return fmt.Errorf("failed to cleanup skip_processed_files in transfer_configs: %w", err)
+			}
+			fmt.Println("Cleaned up invalid values in transfer_configs.skip_processed_files.")
+
+			// Target: notification_services.is_enabled
+			sql = `UPDATE notification_services
+					SET is_enabled = 0
+					WHERE is_enabled IS NOT NULL AND is_enabled NOT IN (0, 1);`
+			if err := tx.Exec(sql).Error; err != nil {
+				// Check if the table exists before failing hard
+				var tableExists int
+				tx.Raw("SELECT count(*) FROM sqlite_master WHERE type='table' AND name='notification_services'").Scan(&tableExists)
+				if tableExists == 0 {
+					fmt.Println("Skipping cleanup for notification_services.is_enabled: table does not exist.")
+				} else {
+					return fmt.Errorf("failed to cleanup is_enabled in notification_services: %w", err)
+				}
+			} else {
+				fmt.Println("Cleaned up invalid values in notification_services.is_enabled.")
+			}
+
+			// Target: auth_providers.enabled
+			sql = `UPDATE auth_providers
+					SET enabled = 0
+					WHERE enabled IS NOT NULL AND enabled NOT IN (0, 1);`
+			if err := tx.Exec(sql).Error; err != nil {
+				// Check if the table exists before failing hard
+				var tableExists int
+				tx.Raw("SELECT count(*) FROM sqlite_master WHERE type='table' AND name='auth_providers'").Scan(&tableExists)
+				if tableExists == 0 {
+					fmt.Println("Skipping cleanup for auth_providers.enabled: table does not exist.")
+				} else {
+					return fmt.Errorf("failed to cleanup enabled in auth_providers: %w", err)
+				}
+			} else {
+				fmt.Println("Cleaned up invalid values in auth_providers.enabled.")
+			}
+
+			fmt.Println("Migration 013 completed successfully.")
+			return nil
+		},
+		Rollback: func(tx *gorm.DB) error {
+			// This migration cleans up data. Rolling back doesn't make sense
+			// as we don't know the original invalid values.
+			fmt.Println("Rollback for migration 013_cleanup_invalid_booleans is not applicable.")
+			return nil
+		},
+	}
+}

internal/db/migrations/migrations.go

@@ -11,18 +11,20 @@ var migrations []*gormigrate.Migration
 func GetMigrations(db *gorm.DB) *gormigrate.Gormigrate {
 	// Add all migrations in order
 	migrations = append(migrations,
-		InitialSchema(),               // 001
-		UpdateGDriveType(),            // 002
-		Add2FA(),                      // 003
-		AddAuditLogs(),                // 004
-		AddDefaultRoles(),             // 005
-		AddTimestampsToJobHistories(), // 006
-		AddNotificationServices(),     // 007
-		AddUserNotifications(),        // 008
-		AddRcloneTables(),             // 009
-		AddRcloneCommandToConfig(),    // 010
-		AddAuthProviders(),            // 011
-		AlterBooleanDefaults(),        // 012
+		InitialSchema(),                // 001
+		UpdateGDriveType(),             // 002
+		Add2FA(),                       // 003
+		AddAuditLogs(),                 // 004
+		AddDefaultRoles(),              // 005
+		AddTimestampsToJobHistories(),  // 006
+		AddNotificationServices(),      // 007
+		AddUserNotifications(),         // 008
+		AddRcloneTables(),              // 009
+		AddRcloneCommandToConfig(),     // 010
+		AddAuthProviders(),             // 011
+		RecoverTransferConfigsRename(), // 011a
+		AlterBooleanDefaults(),         // 012
+		CleanupInvalidBooleans(),       // 013
 	)
 
 	return gormigrate.New(db, gormigrate.DefaultOptions, migrations)

internal/scheduler/scheduler.go

@@ -3,7 +3,6 @@ package scheduler
 import (
 	"context"
 	"fmt"
-	"strings"
 	"sync"
 
 	// Needed for Job.NextRun update
@@ -162,38 +161,18 @@ func (s *Scheduler) ScheduleJob(job *db.Job) error {
 		return nil
 	}
 
-	// Use a 6-field parser for validation and determine the schedule string to use.
-	scheduleToUse := job.Schedule
-	parser := cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
-	_, err := parser.Parse(scheduleToUse)
-
-	// If initial parse fails AND it was a 5-field schedule, try prepending seconds
-	if err != nil && len(strings.Fields(job.Schedule)) == 5 {
-		scheduleWithSeconds := "0 " + job.Schedule
-		_, errSeconds := parser.Parse(scheduleWithSeconds)
-		if errSeconds == nil {
-			scheduleToUse = scheduleWithSeconds // Use the 6-field version
-			err = nil                           // Clear the original error
-			s.logger.LogDebug("Converted 5-field schedule '%s' to 6-field '%s'", job.Schedule, scheduleToUse)
-		}
-	}
-
-	// If error still exists after trying conversion, return it
-	if err != nil {
-		return fmt.Errorf("invalid cron expression '%s': %w", job.Schedule, err)
-	}
-
-	s.logger.LogDebug("Validated cron expression '%s' for job %d", scheduleToUse, jobID)
-
-	// Schedule the job using the validated scheduleToUse
+	// Rely on the cron instance's AddFunc for validation based on its configuration (5 or 6 fields)
+	scheduleToUse := job.Schedule // Use the original schedule string
+	s.logger.LogDebug("Using schedule '%s' for job %d", scheduleToUse, jobID)
+	// Schedule the job using the original schedule string. AddFunc will validate it.
 	entryID, err := s.cron.AddFunc(scheduleToUse, func() { // Calls interface method
 		s.executor.executeJob(jobID) // Calls interface method
 	})
-
 	if err != nil {
-		s.logger.LogError("Error scheduling job %d: %v", jobID, err)
-		return err
-	} // <-- Added missing closing brace
+		// Log and return a more informative error if AddFunc fails validation
+		s.logger.LogError("Error scheduling job %d with schedule '%s': %v", jobID, scheduleToUse, err)
+		return fmt.Errorf("invalid cron expression '%s' for the configured scheduler: %w", scheduleToUse, err)
+	}
 	s.logger.LogDebug("Scheduled job %d with cron entry ID %d", jobID, entryID)
 
 	// Store mapping of job ID to cron entry ID

internal/web/handlers/admin_handlers.go

@@ -1181,15 +1181,18 @@ func (h *Handlers) HandleDeleteUser(c *gin.Context) {
 
 	// Check if this is an admin user
 	if user.GetIsAdmin() {
-		// Count how many admins there are
-		var adminCount int64
-		if err := h.DB.Model(&db.User{}).Where("metadata->>'is_admin' = 'true'").Count(&adminCount).Error; err != nil {
-			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to check admin count"})
+		// Check if other administrators exist
+		var otherAdminCount int64
+		// Use the actual 'is_admin' column, comparing against true
+		if err := h.DB.Model(&db.User{}).
+			Where("is_admin = ? AND id != ?", true, user.ID).
+			Count(&otherAdminCount).Error; err != nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to check for other administrators"})
 			return
 		}
 
-		// If this is the last admin, prevent deletion
-		if adminCount <= 1 {
+		// If no other administrators exist, prevent deletion
+		if otherAdminCount == 0 {
 			c.JSON(http.StatusBadRequest, gin.H{"error": "Cannot delete the last administrator"})
 			return
 		}