Implement jails migration if there are old jails in the jail.local file, before adding the f2b server to f2b-UI

internal/fail2ban/connector_local.go

@@ -147,9 +147,18 @@ func (lc *LocalConnector) Reload(ctx context.Context) error {
 		// Include the output in the error message for better debugging
 		return fmt.Errorf("fail2ban reload error: %w (output: %s)", err, strings.TrimSpace(out))
 	}
+
 	// Check if output indicates success (fail2ban-client returns "OK" on success)
-	if strings.TrimSpace(out) != "OK" && strings.TrimSpace(out) != "" {
+	outputTrimmed := strings.TrimSpace(out)
+	if outputTrimmed != "OK" && outputTrimmed != "" {
 		config.DebugLog("fail2ban reload output: %s", out)
+
+		// Check for jail errors in output even when command succeeds
+		// Look for patterns like "Errors in jail 'jailname'. Skipping..."
+		if strings.Contains(out, "Errors in jail") || strings.Contains(out, "Unable to read the filter") {
+			// Return an error that includes the output so handler can parse it
+			return fmt.Errorf("fail2ban reload completed but with errors (output: %s)", strings.TrimSpace(out))
+		}
 	}
 	return nil
 }
@@ -285,6 +294,9 @@ func (lc *LocalConnector) UpdateDefaultSettings(ctx context.Context, settings co
 
 // EnsureJailLocalStructure implements Connector.
 func (lc *LocalConnector) EnsureJailLocalStructure(ctx context.Context) error {
+	// Note: Migration is handled in newConnectorForServer() before
+	// config.EnsureLocalFail2banAction() is called, so migration has already
+	// run by the time this method is called.
 	return config.EnsureJailLocalStructure()
 }
 

internal/fail2ban/connector_ssh.go

@@ -13,6 +13,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/swissmakers/fail2ban-ui/internal/config"
 )
@@ -1241,10 +1242,112 @@ action = %%(action_mwlg)s
 PY`, escapeForShell(jailLocalPath), escapeForShell(ignoreIPStr), escapeForShell(banactionVal), escapeForShell(banactionAllportsVal), escapeForShell(config.JailLocalBanner()), settings.BantimeIncrement,
 		escapeForShell(settings.Bantime), escapeForShell(settings.Findtime), settings.Maxretry, escapeForShell(settings.Destemail))
 
+	// IMPORTANT: Run migration FIRST before ensuring structure
+	// This is because ensureJailLocalStructure may overwrite jail.local,
+	// which would destroy any jail sections that need to be migrated
+	if err := sc.MigrateJailsFromJailLocalRemote(ctx); err != nil {
+		config.DebugLog("Warning: No migration done (may be normal if no jails to migrate): %v", err)
+		// Don't fail - continue with ensuring structure
+	}
+
+	// Then ensure the basic structure
 	_, err := sc.runRemoteCommand(ctx, []string{"bash", "-lc", ensureScript})
 	return err
 }
 
+// MigrateJailsFromJailLocalRemote migrates non-commented jail sections from jail.local to jail.d/*.local files on remote system.
+func (sc *SSHConnector) MigrateJailsFromJailLocalRemote(ctx context.Context) error {
+	jailLocalPath := "/etc/fail2ban/jail.local"
+	jailDPath := "/etc/fail2ban/jail.d"
+
+	// Check if jail.local exists
+	checkScript := fmt.Sprintf("test -f %s && echo 'exists' || echo 'notfound'", jailLocalPath)
+	out, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", checkScript})
+	if err != nil || strings.TrimSpace(out) != "exists" {
+		return nil // Nothing to migrate
+	}
+
+	// Read jail.local content
+	content, err := sc.runRemoteCommand(ctx, []string{"cat", jailLocalPath})
+	if err != nil {
+		return fmt.Errorf("failed to read jail.local: %w", err)
+	}
+
+	// Parse content locally to extract non-commented sections
+	sections, defaultContent, err := parseJailSectionsUncommented(content)
+	if err != nil {
+		return fmt.Errorf("failed to parse jail.local: %w", err)
+	}
+
+	// If no non-commented, non-DEFAULT jails found, nothing to migrate
+	if len(sections) == 0 {
+		config.DebugLog("No jails to migrate from jail.local on remote system")
+		return nil
+	}
+
+	// Create backup
+	backupPath := jailLocalPath + ".backup." + fmt.Sprintf("%d", time.Now().Unix())
+	backupScript := fmt.Sprintf("cp %s %s", jailLocalPath, backupPath)
+	if _, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", backupScript}); err != nil {
+		return fmt.Errorf("failed to create backup: %w", err)
+	}
+	config.DebugLog("Created backup of jail.local at %s on remote system", backupPath)
+
+	// Ensure jail.d directory exists
+	ensureDirScript := fmt.Sprintf("mkdir -p %s", jailDPath)
+	if _, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", ensureDirScript}); err != nil {
+		return fmt.Errorf("failed to create jail.d directory: %w", err)
+	}
+
+	// Write each jail to its own .local file
+	migratedCount := 0
+	for jailName, jailContent := range sections {
+		if jailName == "" {
+			continue
+		}
+
+		jailFilePath := fmt.Sprintf("%s/%s.local", jailDPath, jailName)
+
+		// Check if .local file already exists
+		checkFileScript := fmt.Sprintf("test -f %s && echo 'exists' || echo 'notfound'", jailFilePath)
+		fileOut, err := sc.runRemoteCommand(ctx, []string{"sh", "-c", checkFileScript})
+		if err == nil && strings.TrimSpace(fileOut) == "exists" {
+			config.DebugLog("Skipping migration for jail %s: .local file already exists", jailName)
+			continue
+		}
+
+		// Write jail content to .local file using heredoc
+		// Escape single quotes in content for shell
+		escapedContent := strings.ReplaceAll(jailContent, "'", "'\"'\"'")
+		writeScript := fmt.Sprintf(`cat > %s <<'JAILEOF'
+%s
+JAILEOF
+`, jailFilePath, escapedContent)
+		if _, err := sc.runRemoteCommand(ctx, []string{"bash", "-c", writeScript}); err != nil {
+			return fmt.Errorf("failed to write jail file %s: %w", jailFilePath, err)
+		}
+		config.DebugLog("Migrated jail %s to %s on remote system", jailName, jailFilePath)
+		migratedCount++
+	}
+
+	// Only rewrite jail.local if we migrated something
+	if migratedCount > 0 {
+		// Rewrite jail.local with only DEFAULT section
+		// Escape single quotes in defaultContent for shell
+		escapedDefault := strings.ReplaceAll(defaultContent, "'", "'\"'\"'")
+		writeLocalScript := fmt.Sprintf(`cat > %s <<'LOCALEOF'
+%s
+LOCALEOF
+`, jailLocalPath, escapedDefault)
+		if _, err := sc.runRemoteCommand(ctx, []string{"bash", "-c", writeLocalScript}); err != nil {
+			return fmt.Errorf("failed to rewrite jail.local: %w", err)
+		}
+		config.DebugLog("Migration completed on remote system: moved %d jails to jail.d/", migratedCount)
+	}
+
+	return nil
+}
+
 // parseJailConfigContent parses jail configuration content and returns JailInfo slice.
 func parseJailConfigContent(content string) []JailInfo {
 	var jails []JailInfo

internal/fail2ban/jail_management.go

@@ -8,6 +8,7 @@ import (
 	"regexp"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/swissmakers/fail2ban-ui/internal/config"
 )
@@ -466,6 +467,236 @@ func parseJailSections(content string) (map[string]string, string, error) {
 	return sections, defaultContent.String(), scanner.Err()
 }
 
+// parseJailSectionsUncommented parses jail.local content and returns:
+// - map of jail name to jail content (excluding DEFAULT, INCLUDES, and commented sections)
+// - DEFAULT section content (including commented lines)
+// Only extracts non-commented jail sections
+func parseJailSectionsUncommented(content string) (map[string]string, string, error) {
+	sections := make(map[string]string)
+	var defaultContent strings.Builder
+
+	// Sections that should be ignored (not jails)
+	ignoredSections := map[string]bool{
+		"DEFAULT":  true,
+		"INCLUDES": true,
+	}
+
+	scanner := bufio.NewScanner(strings.NewReader(content))
+	var currentSection string
+	var currentContent strings.Builder
+	inDefault := false
+	sectionIsCommented := false
+
+	for scanner.Scan() {
+		line := scanner.Text()
+		trimmed := strings.TrimSpace(line)
+
+		// Check if this is a section header
+		if strings.HasPrefix(trimmed, "[") && strings.HasSuffix(trimmed, "]") {
+			// Check if the section is commented
+			originalLine := strings.TrimSpace(line)
+			isCommented := strings.HasPrefix(originalLine, "#")
+
+			// Save previous section
+			if currentSection != "" {
+				sectionContent := strings.TrimSpace(currentContent.String())
+				if inDefault {
+					// Always include DEFAULT section content (even if commented)
+					defaultContent.WriteString(sectionContent)
+					if !strings.HasSuffix(sectionContent, "\n") {
+						defaultContent.WriteString("\n")
+					}
+				} else if !ignoredSections[currentSection] && !sectionIsCommented {
+					// Only save non-commented, non-ignored sections
+					sections[currentSection] = sectionContent
+				}
+			}
+
+			// Start new section
+			if isCommented {
+				// Remove the # from the section name
+				sectionName := strings.Trim(trimmed, "[]")
+				if strings.HasPrefix(sectionName, "#") {
+					sectionName = strings.TrimSpace(strings.TrimPrefix(sectionName, "#"))
+				}
+				currentSection = sectionName
+				sectionIsCommented = true
+			} else {
+				currentSection = strings.Trim(trimmed, "[]")
+				sectionIsCommented = false
+			}
+			currentContent.Reset()
+			currentContent.WriteString(line)
+			currentContent.WriteString("\n")
+			inDefault = (currentSection == "DEFAULT")
+		} else {
+			currentContent.WriteString(line)
+			currentContent.WriteString("\n")
+		}
+	}
+
+	// Save final section
+	if currentSection != "" {
+		sectionContent := strings.TrimSpace(currentContent.String())
+		if inDefault {
+			defaultContent.WriteString(sectionContent)
+		} else if !ignoredSections[currentSection] && !sectionIsCommented {
+			// Only save if it's not an ignored section and not commented
+			sections[currentSection] = sectionContent
+		}
+	}
+
+	return sections, defaultContent.String(), scanner.Err()
+}
+
+// MigrateJailsFromJailLocal migrates non-commented jail sections from jail.local to jail.d/*.local files.
+// This should be called when a server is added or enabled to migrate legacy jails.
+func MigrateJailsFromJailLocal() error {
+	localPath := "/etc/fail2ban/jail.local"
+	jailDPath := "/etc/fail2ban/jail.d"
+
+	// Check if jail.local exists
+	if _, err := os.Stat(localPath); os.IsNotExist(err) {
+		return nil // Nothing to migrate
+	}
+
+	// Read jail.local content
+	content, err := os.ReadFile(localPath)
+	if err != nil {
+		return fmt.Errorf("failed to read jail.local: %w", err)
+	}
+
+	// Parse content to extract non-commented sections
+	sections, defaultContent, err := parseJailSectionsUncommented(string(content))
+	if err != nil {
+		return fmt.Errorf("failed to parse jail.local: %w", err)
+	}
+
+	// If no non-commented, non-DEFAULT jails found, nothing to migrate
+	if len(sections) == 0 {
+		config.DebugLog("No jails to migrate from jail.local")
+		return nil
+	}
+
+	// Create backup of jail.local
+	backupPath := localPath + ".backup." + fmt.Sprintf("%d", time.Now().Unix())
+	if err := os.WriteFile(backupPath, content, 0644); err != nil {
+		return fmt.Errorf("failed to create backup: %w", err)
+	}
+	config.DebugLog("Created backup of jail.local at %s", backupPath)
+
+	// Ensure jail.d directory exists
+	if err := os.MkdirAll(jailDPath, 0755); err != nil {
+		return fmt.Errorf("failed to create jail.d directory: %w", err)
+	}
+
+	// Write each jail to its own .local file in jail.d/
+	migratedCount := 0
+	for jailName, jailContent := range sections {
+		// Skip empty jail names
+		if jailName == "" {
+			continue
+		}
+
+		jailFilePath := filepath.Join(jailDPath, jailName+".local")
+
+		// Check if .local file already exists
+		if _, err := os.Stat(jailFilePath); err == nil {
+			// File already exists - skip migration for this jail
+			config.DebugLog("Skipping migration for jail %s: .local file already exists", jailName)
+			continue
+		}
+
+		// Ensure enabled = false is set by default for migrated jails
+		// Check if enabled is already set in the content
+		enabledSet := strings.Contains(jailContent, "enabled") || strings.Contains(jailContent, "Enabled")
+		if !enabledSet {
+			// Add enabled = false at the beginning of the jail section
+			// Find the first line after [jailName]
+			lines := strings.Split(jailContent, "\n")
+			modifiedContent := ""
+			for i, line := range lines {
+				modifiedContent += line + "\n"
+				// After the section header, add enabled = false
+				if i == 0 && strings.HasPrefix(strings.TrimSpace(line), "[") && strings.HasSuffix(strings.TrimSpace(line), "]") {
+					modifiedContent += "enabled = false\n"
+				}
+			}
+			jailContent = modifiedContent
+		} else {
+			// If enabled is set, ensure it's false by replacing any enabled = true
+			jailContent = regexp.MustCompile(`(?m)^\s*enabled\s*=\s*true\s*$`).ReplaceAllString(jailContent, "enabled = false")
+		}
+
+		// Write jail content to .local file
+		if err := os.WriteFile(jailFilePath, []byte(jailContent), 0644); err != nil {
+			return fmt.Errorf("failed to write jail file %s: %w", jailFilePath, err)
+		}
+		config.DebugLog("Migrated jail %s to %s (enabled = false)", jailName, jailFilePath)
+		migratedCount++
+	}
+
+	// Only rewrite jail.local if we actually migrated something
+	if migratedCount > 0 {
+		// Rewrite jail.local with only DEFAULT section and commented jails
+		// We need to preserve commented sections, so we'll reconstruct the file
+		newLocalContent := defaultContent
+
+		// Add back commented sections that weren't migrated
+		scanner := bufio.NewScanner(strings.NewReader(string(content)))
+		var inCommentedJail bool
+		var commentedJailContent strings.Builder
+		var commentedJailName string
+		for scanner.Scan() {
+			line := scanner.Text()
+			trimmed := strings.TrimSpace(line)
+
+			if strings.HasPrefix(trimmed, "[") && strings.HasSuffix(trimmed, "]") {
+				// Check if this is a commented section
+				originalLine := strings.TrimSpace(line)
+				if strings.HasPrefix(originalLine, "#[") {
+					// Save previous commented jail if any
+					if inCommentedJail && commentedJailName != "" {
+						newLocalContent += commentedJailContent.String()
+					}
+					inCommentedJail = true
+					commentedJailContent.Reset()
+					commentedJailName = strings.Trim(trimmed, "[]")
+					if strings.HasPrefix(commentedJailName, "#") {
+						commentedJailName = strings.TrimSpace(strings.TrimPrefix(commentedJailName, "#"))
+					}
+					commentedJailContent.WriteString(line)
+					commentedJailContent.WriteString("\n")
+				} else {
+					// Non-commented section - save previous commented jail if any
+					if inCommentedJail && commentedJailName != "" {
+						newLocalContent += commentedJailContent.String()
+						inCommentedJail = false
+						commentedJailContent.Reset()
+					}
+				}
+			} else if inCommentedJail {
+				commentedJailContent.WriteString(line)
+				commentedJailContent.WriteString("\n")
+			}
+		}
+		// Save final commented jail if any
+		if inCommentedJail && commentedJailName != "" {
+			newLocalContent += commentedJailContent.String()
+		}
+
+		if !strings.HasSuffix(newLocalContent, "\n") {
+			newLocalContent += "\n"
+		}
+		if err := os.WriteFile(localPath, []byte(newLocalContent), 0644); err != nil {
+			return fmt.Errorf("failed to rewrite jail.local: %w", err)
+		}
+		config.DebugLog("Migration completed: moved %d jails to jail.d/", migratedCount)
+	}
+
+	return nil
+}
+
 // parseJailConfigFileOnlyDefault parses only the DEFAULT section from a jail config file.
 func parseJailConfigFileOnlyDefault(path string) ([]JailInfo, error) {
 	var jails []JailInfo

internal/fail2ban/manager.go

@@ -169,6 +169,14 @@ func updateConnectorAction(ctx context.Context, conn Connector) error {
 func newConnectorForServer(server config.Fail2banServer) (Connector, error) {
 	switch server.Type {
 	case "local":
+		// IMPORTANT: Run migration FIRST before ensuring structure
+		// This ensures any legacy jails in jail.local are migrated to jail.d/*.local
+		// before ensureJailLocalStructure() overwrites jail.local
+		if err := MigrateJailsFromJailLocal(); err != nil {
+			config.DebugLog("Warning: migration check failed (may be normal if no jails to migrate): %v", err)
+			// Don't fail - continue with ensuring structure
+		}
+
 		if err := config.EnsureLocalFail2banAction(server); err != nil {
 			fmt.Printf("warning: failed to ensure local fail2ban action: %v\n", err)
 		}

pkg/web/handlers.go

@@ -1041,6 +1041,51 @@ func getJailNames(jails map[string]bool) []string {
 	return names
 }
 
+func contains(slice []string, item string) bool {
+	for _, s := range slice {
+		if s == item {
+			return true
+		}
+	}
+	return false
+}
+
+// parseJailErrorsFromReloadOutput extracts jail names that have errors from reload output.
+// Looks for patterns like "Errors in jail 'jailname'. Skipping..." or "Unable to read the filter 'filtername'"
+func parseJailErrorsFromReloadOutput(output string) []string {
+	var problematicJails []string
+	lines := strings.Split(output, "\n")
+
+	for _, line := range lines {
+		// Look for "Errors in jail 'jailname'. Skipping..."
+		if strings.Contains(line, "Errors in jail") && strings.Contains(line, "Skipping") {
+			// Extract jail name between single quotes
+			re := regexp.MustCompile(`Errors in jail '([^']+)'`)
+			matches := re.FindStringSubmatch(line)
+			if len(matches) > 1 {
+				problematicJails = append(problematicJails, matches[1])
+			}
+		}
+		// Also check for filter errors that might indicate jail problems
+		// "Unable to read the filter 'filtername'" - this might be referenced by a jail
+		// Note: Filter errors are often associated with jails, but we primarily track
+		// jail errors directly via "Errors in jail" messages above
+		_ = strings.Contains(line, "Unable to read the filter") // Track for future enhancement
+	}
+
+	// Remove duplicates
+	seen := make(map[string]bool)
+	uniqueJails := []string{}
+	for _, jail := range problematicJails {
+		if !seen[jail] {
+			seen[jail] = true
+			uniqueJails = append(uniqueJails, jail)
+		}
+	}
+
+	return uniqueJails
+}
+
 // After updating, fail2ban is reloaded to apply the changes.
 func UpdateJailManagementHandler(c *gin.Context) {
 	config.DebugLog("----------------------------")
@@ -1081,9 +1126,75 @@ func UpdateJailManagementHandler(c *gin.Context) {
 	config.DebugLog("Successfully updated jail enabled states")
 
 	// Reload fail2ban to apply the changes (reload is sufficient for jail enable/disable)
-	if err := conn.Reload(c.Request.Context()); err != nil {
-		config.DebugLog("Error: failed to reload fail2ban after updating jail settings: %v", err)
-		errMsg := err.Error()
+	reloadErr := conn.Reload(c.Request.Context())
+
+	// Check for errors in reload output even if reload "succeeded"
+	var problematicJails []string
+	var detailedErrorOutput string
+	if reloadErr != nil {
+		errMsg := reloadErr.Error()
+		config.DebugLog("Error: failed to reload fail2ban after updating jail settings: %v", reloadErr)
+
+		// Extract output from error message (format: "fail2ban reload completed but with errors (output: ...)")
+		if strings.Contains(errMsg, "(output:") {
+			// Extract the output part
+			outputStart := strings.Index(errMsg, "(output:") + 8
+			outputEnd := strings.LastIndex(errMsg, ")")
+			if outputEnd > outputStart {
+				detailedErrorOutput = errMsg[outputStart:outputEnd]
+				problematicJails = parseJailErrorsFromReloadOutput(detailedErrorOutput)
+			}
+		} else if strings.Contains(errMsg, "output:") {
+			// Alternative format: "fail2ban reload error: ... (output: ...)"
+			outputStart := strings.Index(errMsg, "output:") + 7
+			if outputStart < len(errMsg) {
+				detailedErrorOutput = strings.TrimSpace(errMsg[outputStart:])
+				problematicJails = parseJailErrorsFromReloadOutput(detailedErrorOutput)
+			}
+		}
+
+		// If we found problematic jails, disable them
+		if len(problematicJails) > 0 {
+			config.DebugLog("Found %d problematic jail(s) in reload output: %v", len(problematicJails), problematicJails)
+
+			// Create disable update for problematic jails
+			disableUpdate := make(map[string]bool)
+			for _, jailName := range problematicJails {
+				disableUpdate[jailName] = false
+			}
+
+			// Also disable any jails that were enabled in this request if they're in the problematic list
+			for jailName := range enabledJails {
+				if contains(problematicJails, jailName) {
+					disableUpdate[jailName] = false
+				}
+			}
+
+			if len(disableUpdate) > 0 {
+				if disableErr := conn.UpdateJailEnabledStates(c.Request.Context(), disableUpdate); disableErr != nil {
+					config.DebugLog("Error disabling problematic jails: %v", disableErr)
+				} else {
+					// Reload again after disabling
+					if reloadErr2 := conn.Reload(c.Request.Context()); reloadErr2 != nil {
+						config.DebugLog("Error: failed to reload fail2ban after disabling problematic jails: %v", reloadErr2)
+					}
+				}
+			}
+
+			// Update enabledJails to include problematic jails for response
+			for _, jailName := range problematicJails {
+				enabledJails[jailName] = true
+			}
+		}
+
+		// Update errMsg with detailed error output when debug mode is enabled
+		settings := config.GetSettings()
+		if settings.Debug && detailedErrorOutput != "" {
+			errMsg = fmt.Sprintf("%s\n\nDetailed error output:\n%s", errMsg, detailedErrorOutput)
+		} else if detailedErrorOutput != "" {
+			// Even without debug mode, include basic error info
+			errMsg = fmt.Sprintf("%s (check debug mode for details)", errMsg)
+		}
 
 		// If any jails were enabled in this request and reload failed, disable them all
 		if len(enabledJails) > 0 {
@@ -1106,7 +1217,7 @@ func UpdateJailManagementHandler(c *gin.Context) {
 			}
 
 			// Reload again after disabling
-			if reloadErr := conn.Reload(c.Request.Context()); reloadErr != nil {
+			if reloadErr = conn.Reload(c.Request.Context()); reloadErr != nil {
 				config.DebugLog("Error: failed to reload fail2ban after disabling jails: %v", reloadErr)
 				c.JSON(http.StatusOK, gin.H{
 					"error":        fmt.Sprintf("Failed to reload fail2ban after disabling jails: %v", reloadErr),