Check jail.local state and warn user if it is not fail2ban-UI managed, disable automatic jail.local migration because it is only testing

internal/fail2ban/connector_agent.go

@@ -443,11 +443,31 @@ func (ac *AgentConnector) UpdateDefaultSettings(ctx context.Context, settings co
 	return ac.put(ctx, "/v1/jails/default-settings", payload, nil)
 }
 
+// CheckJailLocalIntegrity implements Connector.
+func (ac *AgentConnector) CheckJailLocalIntegrity(ctx context.Context) (bool, bool, error) {
+	var result struct {
+		Exists      bool `json:"exists"`
+		HasUIAction bool `json:"hasUIAction"`
+	}
+	if err := ac.get(ctx, "/v1/jails/check-integrity", &result); err != nil {
+		// If the agent does not implement this endpoint, assume OK
+		if strings.Contains(err.Error(), "404") || strings.Contains(err.Error(), "not found") {
+			return false, false, nil
+		}
+		return false, false, fmt.Errorf("failed to check jail.local integrity on %s: %w", ac.server.Name, err)
+	}
+	return result.Exists, result.HasUIAction, nil
+}
+
 // EnsureJailLocalStructure implements Connector.
 func (ac *AgentConnector) EnsureJailLocalStructure(ctx context.Context) error {
-	// Call agent API endpoint to ensure jail.local structure
-	// If the endpoint doesn't exist, we'll need to implement it on the agent side
-	// For now, we'll try calling it and handle the error gracefully
+	// Safety: if jail.local exists but is not managed by Fail2ban-UI,
+	// it belongs to the user; never overwrite it.
+	if exists, hasUI, err := ac.CheckJailLocalIntegrity(ctx); err == nil && exists && !hasUI {
+		config.DebugLog("jail.local on agent server %s exists but is not managed by Fail2ban-UI -- skipping overwrite", ac.server.Name)
+		return nil
+	}
+
 	return ac.post(ctx, "/v1/jails/ensure-structure", nil, nil)
 }
 

internal/fail2ban/connector_local.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 	"os/exec"
 	"sort"
 	"strings"
@@ -343,9 +344,6 @@ 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()
 }
 
@@ -369,6 +367,20 @@ func (lc *LocalConnector) DeleteFilter(ctx context.Context, filterName string) e
 	return DeleteFilter(filterName)
 }
 
+// CheckJailLocalIntegrity implements Connector.
+func (lc *LocalConnector) CheckJailLocalIntegrity(ctx context.Context) (bool, bool, error) {
+	const jailLocalPath = "/etc/fail2ban/jail.local"
+	content, err := os.ReadFile(jailLocalPath)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, false, nil // file does not exist; OK, will be created
+		}
+		return false, false, fmt.Errorf("failed to read jail.local: %w", err)
+	}
+	hasUIAction := strings.Contains(string(content), "ui-custom-action")
+	return true, hasUIAction, nil
+}
+
 func executeShellCommand(ctx context.Context, command string) (string, error) {
 	parts := strings.Fields(command)
 	if len(parts) == 0 {

internal/fail2ban/connector_ssh.go

@@ -20,6 +20,8 @@ import (
 	"github.com/swissmakers/fail2ban-ui/internal/config"
 )
 
+// sshEnsureActionScript only deploys action.d/ui-custom-action.conf.
+// jail.local is managed by EnsureJailLocalStructure
 const sshEnsureActionScript = `python3 - <<'PY'
 import base64
 import pathlib
@@ -30,40 +32,6 @@ try:
     action_dir.mkdir(parents=True, exist_ok=True)
     action_cfg = base64.b64decode("__PAYLOAD__").decode("utf-8")
     (action_dir / "ui-custom-action.conf").write_text(action_cfg)
-    
-    jail_file = pathlib.Path("/etc/fail2ban/jail.local")
-    if not jail_file.exists():
-        jail_file.write_text("[DEFAULT]\n")
-    
-    lines = jail_file.read_text().splitlines()
-    already = any("Custom Fail2Ban action applied by fail2ban-ui" in line for line in lines)
-    if not already:
-        new_lines = []
-        inserted = False
-        for line in lines:
-            stripped = line.strip()
-            if stripped.startswith("action") and "ui-custom-action" not in stripped and not inserted:
-                if not stripped.startswith("#"):
-                    new_lines.append("# " + line)
-                else:
-                    new_lines.append(line)
-                new_lines.append("# Custom Fail2Ban action applied by fail2ban-ui")
-                new_lines.append("action = %(action_mwlg)s")
-                inserted = True
-                continue
-            new_lines.append(line)
-        if not inserted:
-            insert_at = None
-            for idx, value in enumerate(new_lines):
-                if value.strip().startswith("[DEFAULT]"):
-                    insert_at = idx + 1
-                    break
-            if insert_at is None:
-                new_lines.append("[DEFAULT]")
-                insert_at = len(new_lines)
-            new_lines.insert(insert_at, "# Custom Fail2Ban action applied by fail2ban-ui")
-            new_lines.insert(insert_at + 1, "action = %(action_mwlg)s")
-        jail_file.write_text("\n".join(new_lines) + "\n")
 except Exception as e:
     sys.stderr.write(f"Error: {e}\n")
     sys.exit(1)
@@ -1785,22 +1753,53 @@ PY`, escapeForShell(jailLocalPath), escapeForShell(ignoreIPStr), escapeForShell(
 	return err
 }
 
+// CheckJailLocalIntegrity implements Connector.
+func (sc *SSHConnector) CheckJailLocalIntegrity(ctx context.Context) (bool, bool, error) {
+	const jailLocalPath = "/etc/fail2ban/jail.local"
+	output, err := sc.runRemoteCommand(ctx, []string{"cat", jailLocalPath})
+	if err != nil {
+		// "No such file" means jail.local does not exist – that's fine
+		if strings.Contains(err.Error(), "No such file") || strings.Contains(output, "No such file") {
+			return false, false, nil
+		}
+		return false, false, fmt.Errorf("failed to read jail.local on %s: %w", sc.server.Name, err)
+	}
+	hasUIAction := strings.Contains(output, "ui-custom-action")
+	return true, hasUIAction, nil
+}
+
 // EnsureJailLocalStructure implements Connector.
-// For SSH connectors we:
-//  1. Migrate any legacy jails out of jail.local into jail.d/*.local
-//  2. Rebuild /etc/fail2ban/jail.local with a clean, managed structure
-//     (banner, [DEFAULT] section based on current settings, and action_mwlg/action override).
+// If JAIL_AUTOMIGRATION=true, it first migrates any legacy jails to jail.d/.
 func (sc *SSHConnector) EnsureJailLocalStructure(ctx context.Context) error {
 	jailLocalPath := "/etc/fail2ban/jail.local"
+
+	// Check whether jail.local already exists and whether it belongs to us
+	exists, hasUI, chkErr := sc.CheckJailLocalIntegrity(ctx)
+	if chkErr != nil {
+		config.DebugLog("Warning: could not check jail.local integrity on %s: %v", sc.server.Name, chkErr)
+		// Proceed cautiously; treat as "not ours" if the check itself failed.
+	}
+	if exists && !hasUI {
+		// The file belongs to the user; never overwrite it.
+		config.DebugLog("jail.local on server %s exists but is not managed by Fail2ban-UI - skipping overwrite", sc.server.Name)
+		return nil
+	}
+
+	// Run experimental migration if enabled
+	if isJailAutoMigrationEnabled() {
+		config.DebugLog("JAIL_AUTOMIGRATION=true: running experimental jail.local → jail.d/ migration for SSH server %s", sc.server.Name)
+		if err := sc.MigrateJailsFromJailLocalRemote(ctx); err != nil {
+			return fmt.Errorf("failed to migrate legacy jails from jail.local on remote server %s: %w", sc.server.Name, err)
+		}
+	}
+
+	// Build the managed jail.local content
 	settings := config.GetSettings()
 
-	// Convert IgnoreIPs array to space-separated string
 	ignoreIPStr := strings.Join(settings.IgnoreIPs, " ")
 	if ignoreIPStr == "" {
 		ignoreIPStr = "127.0.0.1/8 ::1"
 	}
-
-	// Set default banaction values if not set
 	banactionVal := settings.Banaction
 	if banactionVal == "" {
 		banactionVal = "nftables-multiport"
@@ -1814,7 +1813,6 @@ func (sc *SSHConnector) EnsureJailLocalStructure(ctx context.Context) error {
 		chainVal = "INPUT"
 	}
 
-	// Build the new jail.local content in Go (mirrors local ensureJailLocalStructure)
 	banner := config.JailLocalBanner()
 
 	defaultSection := fmt.Sprintf(`[DEFAULT]
@@ -1859,15 +1857,6 @@ action = %(action_mwlg)s
 	// Escape single quotes for safe use in a single-quoted heredoc
 	escaped := strings.ReplaceAll(content, "'", "'\"'\"'")
 
-	// 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 migration fails for any reason, we SHOULD NOT overwrite jail.local,
-	// otherwise legacy jails would be lost.
-	if err := sc.MigrateJailsFromJailLocalRemote(ctx); err != nil {
-		return fmt.Errorf("failed to migrate legacy jails from jail.local on remote server %s: %w", sc.server.Name, err)
-	}
-
 	// Write the rebuilt content via heredoc over SSH
 	writeScript := fmt.Sprintf(`cat > %s <<'JAILLOCAL'
 %s
@@ -1879,6 +1868,7 @@ JAILLOCAL
 }
 
 // MigrateJailsFromJailLocalRemote migrates non-commented jail sections from jail.local to jail.d/*.local files on remote system.
+// EXPERIMENTAL: Only called when JAIL_AUTOMIGRATION=true. It is always best to migrate a pre-existing jail.local by hand.
 func (sc *SSHConnector) MigrateJailsFromJailLocalRemote(ctx context.Context) error {
 	jailLocalPath := "/etc/fail2ban/jail.local"
 	jailDPath := "/etc/fail2ban/jail.d"
@@ -1946,7 +1936,7 @@ func (sc *SSHConnector) MigrateJailsFromJailLocalRemote(ctx context.Context) err
 		writeScript := fmt.Sprintf(`cat > %s <<'JAILEOF'
 %s
 JAILEOF
-'`, jailFilePath, escapedContent)
+`, jailFilePath, escapedContent)
 		if _, err := sc.runRemoteCommand(ctx, []string{writeScript}); err != nil {
 			return fmt.Errorf("failed to write jail file %s: %w", jailFilePath, err)
 		}
@@ -1962,7 +1952,7 @@ JAILEOF
 		writeLocalScript := fmt.Sprintf(`cat > %s <<'LOCALEOF'
 %s
 LOCALEOF
-'`, jailLocalPath, escapedDefault)
+`, jailLocalPath, escapedDefault)
 		if _, err := sc.runRemoteCommand(ctx, []string{writeLocalScript}); err != nil {
 			return fmt.Errorf("failed to rewrite jail.local: %w", err)
 		}

internal/fail2ban/jail_management.go

@@ -17,6 +17,12 @@ var (
 	migrationOnce sync.Once
 )
 
+// Auto-migration of an existing jail.local into jail.d/ is experimental and disabled by default;
+// it is always best to migrate a pre-existing jail.local by hand.
+func isJailAutoMigrationEnabled() bool {
+	return strings.EqualFold(os.Getenv("JAIL_AUTOMIGRATION"), "true")
+}
+
 // ensureJailLocalFile ensures that a .local file exists for the given jail.
 // If .local doesn't exist, it copies from .conf if available, or creates a minimal section.
 func ensureJailLocalFile(jailName string) error {
@@ -324,16 +330,17 @@ func DeleteJail(jailName string) error {
 	return nil
 }
 
-// GetAllJails reads jails from /etc/fail2ban/jail.local (DEFAULT only) and /etc/fail2ban/jail.d directory.
-// Automatically migrates legacy jails from jail.local to jail.d on first call.
-// Now uses DiscoverJailsFromFiles() for file-based discovery.
+// GetAllJails reads jails from /etc/fail2ban/jail.d directory.
 func GetAllJails() ([]JailInfo, error) {
-	// Run migration once if needed
-	migrationOnce.Do(func() {
-		if err := MigrateJailsToJailD(); err != nil {
-			config.DebugLog("Migration warning: %v", err)
-		}
-	})
+	// Run migration once if enabled (experimental, off by default)
+	if isJailAutoMigrationEnabled() {
+		migrationOnce.Do(func() {
+			config.DebugLog("JAIL_AUTOMIGRATION=true: running experimental jail.local → jail.d/ migration")
+			if err := MigrateJailsFromJailLocal(); err != nil {
+				config.DebugLog("Migration warning: %v", err)
+			}
+		})
+	}
 
 	// Discover jails from filesystem
 	jails, err := DiscoverJailsFromFiles()
@@ -510,142 +517,9 @@ func UpdateJailEnabledStates(updates map[string]bool) error {
 	return nil
 }
 
-// MigrateJailsToJailD migrates all non-DEFAULT jails from jail.local to individual files in jail.d/.
-// Creates a backup of jail.local before migration. If a jail already exists in jail.d, jail.local takes precedence.
-func MigrateJailsToJailD() 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 sections
-	sections, defaultContent, err := parseJailSections(string(content))
-	if err != nil {
-		return fmt.Errorf("failed to parse jail.local: %w", err)
-	}
-
-	// If no non-DEFAULT jails found, nothing to migrate
-	if len(sections) == 0 {
-		return nil
-	}
-
-	// Create backup of jail.local
-	backupPath := localPath + ".backup." + fmt.Sprintf("%d", os.Getpid())
-	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 file in jail.d/
-	for jailName, jailContent := range sections {
-		jailFilePath := filepath.Join(jailDPath, jailName+".conf")
-
-		// Check if file already exists
-		if _, err := os.Stat(jailFilePath); err == nil {
-			// File exists - jail.local takes precedence, so overwrite
-			config.DebugLog("Overwriting existing jail file %s with content from jail.local", jailFilePath)
-		}
-
-		// Write jail content to file
-		if err := os.WriteFile(jailFilePath, []byte(jailContent), 0644); err != nil {
-			return fmt.Errorf("failed to write jail file %s: %w", jailFilePath, err)
-		}
-	}
-
-	// Rewrite jail.local with only DEFAULT section
-	newLocalContent := defaultContent
-	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/", len(sections))
-	return nil
-}
-
-// parseJailSections parses jail.local content and returns:
-// - map of jail name to jail content (excluding DEFAULT and INCLUDES)
-// - DEFAULT section content
-func parseJailSections(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
-
-	for scanner.Scan() {
-		line := scanner.Text()
-		trimmed := strings.TrimSpace(line)
-
-		if strings.HasPrefix(trimmed, "[") && strings.HasSuffix(trimmed, "]") {
-			// Save previous section
-			if currentSection != "" {
-				sectionContent := strings.TrimSpace(currentContent.String())
-				if inDefault {
-					defaultContent.WriteString(sectionContent)
-					if !strings.HasSuffix(sectionContent, "\n") {
-						defaultContent.WriteString("\n")
-					}
-				} else if !ignoredSections[currentSection] {
-					// Only save if it's not an ignored section
-					sections[currentSection] = sectionContent
-				}
-			}
-
-			// Start new section
-			currentSection = strings.Trim(trimmed, "[]")
-			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] {
-			// Only save if it's not an ignored section
-			sections[currentSection] = sectionContent
-		}
-	}
-
-	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
@@ -725,7 +599,7 @@ func parseJailSectionsUncommented(content string) (map[string]string, string, er
 }
 
 // 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.
+// EXPERIMENTAL: Only called when JAIL_AUTOMIGRATION=true. It is always best to migrate a pre-existing jail.local by hand.
 func MigrateJailsFromJailLocal() error {
 	localPath := "/etc/fail2ban/jail.local"
 	jailDPath := "/etc/fail2ban/jail.d"

internal/fail2ban/manager.go

@@ -43,6 +43,10 @@ type Connector interface {
 	// Jail local structure management
 	EnsureJailLocalStructure(ctx context.Context) error
 
+	// CheckJailLocalIntegrity checks whether jail.local exists and contains the
+	// ui-custom-action marker, which indicates it is managed by Fail2ban-UI.
+	CheckJailLocalIntegrity(ctx context.Context) (bool, bool, error)
+
 	// Jail and filter creation/deletion
 	CreateJail(ctx context.Context, jailName, content string) error
 	DeleteJail(ctx context.Context, jailName string) error
@@ -176,11 +180,13 @@ 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 {
-			return nil, fmt.Errorf("failed to initialise local fail2ban connector for %s: %w", server.Name, err)
+		// Run migration FIRST before ensuring structure — but only when
+		// the experimental JAIL_AUTOMIGRATION=true env var is set.
+		if isJailAutoMigrationEnabled() {
+			config.DebugLog("JAIL_AUTOMIGRATION=true: running experimental jail.local → jail.d/ migration for local server %s", server.Name)
+			if err := MigrateJailsFromJailLocal(); err != nil {
+				return nil, fmt.Errorf("failed to initialise local fail2ban connector for %s: %w", server.Name, err)
+			}
 		}
 
 		if err := config.EnsureLocalFail2banAction(server); err != nil {