Fix 23 code review issues: security, reliability, thread safety, and quality improvements

High priority:
- Add single-instance mutex to prevent concurrent database access
- Configure SQLite WAL mode and busy_timeout for corruption prevention
- Fix path traversal double-encoding bypass with iterative URL decoding
- Handle FormatException in Uri.UnescapeDataString for malformed input
- Add WebView2 runtime presence check with graceful error display
- Add 5-second timeout to connection pool drain to prevent shutdown hangs

Medium priority:
- Unify SessionViewModel connecting host tracking into single ConcurrentDictionary
- Add semaphore eviction to prevent unbounded memory growth in host locks
- Fix ScrollViewer event handler leak in SessionTabStrip (add/remove in Loaded/Unloaded)
- Fix double enumeration of tagIds IEnumerable in HostRepository.SearchAsync
- Marshal IsLoading property changes to UI thread in HostManagementViewModel
- Add IPv6 address validation support using IPAddress.TryParse
- Add schema version tracking to DbMigrator to skip applied migrations
- Copy ACL from original file to backup in KeyEncryptionService
- Sort backup files by filename timestamp instead of unreliable CreationTimeUtc

Low priority:
- Replace Debug.WriteLine with ILogger in ConnectionHistoryViewModel
- Add recursion guard in ApplyFilterAndPaging to prevent re-entrant calls
- Add case-insensitive JsonSerializerOptions for FavoriteStorageModel
- Fix legacy pipe format IPv6 hostname parsing with :/ separator
- Batch QuickConnect ObservableCollection updates to reduce UI flicker
- Forward CancellationToken in TryPingAsync via Task.WhenAny
- Restrict POSIX env var validation to ASCII-only characters
- Optimize ANSI escape stripping to process only tail of output batches
- Add disk space pre-check (100MB minimum) for session recording

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: Tomer Vaknin

CODE_REVIEW.md

@@ -21,13 +21,29 @@ namespace SshManager.App;
 
 public partial class App : Application
 {
+    private static readonly string MutexName = @"Global\SshManager_SingleInstance_B7E3F2A1";
+    private Mutex? _singleInstanceMutex;
     private readonly IHost _host;
 #if DEBUG
     private ITestServer? _testServer;
 #endif
 
     public App()
     {
+        // Enforce single instance before any other initialization
+        _singleInstanceMutex = new Mutex(true, MutexName, out var createdNew);
+        if (!createdNew)
+        {
+            MessageBox.Show(
+                "SshManager is already running.\n\nOnly one instance can run at a time.",
+                "SshManager",
+                MessageBoxButton.OK,
+                MessageBoxImage.Information);
+            Shutdown(0);
+            _host = null!;
+            return;
+        }
+
         // Configure Serilog early for bootstrap logging
         Bootstrapper.ConfigureSerilog();
 
@@ -155,6 +171,14 @@ protected override async void OnExit(ExitEventArgs e)
         }
         finally
         {
+            // Release single-instance mutex so another instance can start
+            if (_singleInstanceMutex != null)
+            {
+                _singleInstanceMutex.ReleaseMutex();
+                _singleInstanceMutex.Dispose();
+                _singleInstanceMutex = null;
+            }
+
             Log.Information("Application exit complete");
             await Log.CloseAndFlushAsync();
         }

src/SshManager.App/App.xaml.cs

@@ -88,6 +88,9 @@ public static class MigrationDefaults
         public const int ShowHostConnectionStats = 1;
         public const int PinFavoritesToTop = 1;
 
+        // Settings dialog UI
+        public const int IsAdvancedMode = 0;
+
         // Performance settings (Phase 1)
         public const int TerminalOutputFlushIntervalMs = 16;
         public const int TerminalOutputMaxBatchSize = 8192;

src/SshManager.App/AppConstants.cs

@@ -1,3 +1,4 @@
+using Microsoft.Data.Sqlite;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.DependencyInjection;
 using SshManager.Data;
@@ -13,13 +14,45 @@ public static class DataServiceExtensions
 {
     public static IServiceCollection AddDataServices(this IServiceCollection services)
     {
-        // Database context factory
+        // Database context factory with WAL mode and busy timeout for concurrent access safety
         services.AddDbContextFactory<AppDbContext>(options =>
         {
             var dbPath = DbPaths.GetDbPath();
-            options.UseSqlite($"Data Source={dbPath}");
+            var connectionString = new SqliteConnectionStringBuilder
+            {
+                DataSource = dbPath,
+                // busy_timeout prevents "database is locked" errors under concurrent access
+                DefaultTimeout = 5
+            }.ToString();
+
+            options.UseSqlite(connectionString, sqliteOptions =>
+            {
+                sqliteOptions.CommandTimeout(30);
+            });
         });
 
+        // Execute WAL mode pragma once at startup via a shared connection.
+        // WAL mode persists across connections once set, so we only need to do this once.
+        {
+            var dbPath = DbPaths.GetDbPath();
+            var pragmaConnection = new SqliteConnection($"Data Source={dbPath}");
+            try
+            {
+                pragmaConnection.Open();
+                using var walCmd = pragmaConnection.CreateCommand();
+                walCmd.CommandText = "PRAGMA journal_mode=WAL;";
+                walCmd.ExecuteNonQuery();
+                using var busyCmd = pragmaConnection.CreateCommand();
+                busyCmd.CommandText = "PRAGMA busy_timeout=5000;";
+                busyCmd.ExecuteNonQuery();
+            }
+            finally
+            {
+                pragmaConnection.Close();
+                pragmaConnection.Dispose();
+            }
+        }
+
         // Repositories
         services.AddSingleton<IHostRepository, HostRepository>();
         services.AddSingleton<IGroupRepository, GroupRepository>();

src/SshManager.App/Infrastructure/DataServiceExtensions.cs

@@ -82,6 +82,12 @@ public static async Task MigrateAsync(AppDbContext db, Serilog.ILogger logger)
         var connection = db.Database.GetDbConnection();
         await connection.OpenAsync();
 
+        // Ensure SchemaVersion table exists and read current version
+        var currentVersion = await GetSchemaVersionAsync(connection);
+        logger.Information("Current schema version: {Version}", currentVersion);
+
+        if (currentVersion < 1)
+        {
         // First, create missing tables
         await CreateMissingTablesAsync(db, connection, logger);
 
@@ -176,6 +182,8 @@ public static async Task MigrateAsync(AppDbContext db, Serilog.ILogger logger)
             ["HostListViewMode"] = ("INTEGER", AppConstants.MigrationDefaults.HostListViewMode.ToString()),  // Normal = 1
             ["ShowHostConnectionStats"] = ("INTEGER", AppConstants.MigrationDefaults.ShowHostConnectionStats.ToString()),
             ["PinFavoritesToTop"] = ("INTEGER", AppConstants.MigrationDefaults.PinFavoritesToTop.ToString()),
+            // Settings dialog UI state
+            ["IsAdvancedMode"] = ("INTEGER", AppConstants.MigrationDefaults.IsAdvancedMode.ToString()),
             // Performance settings (Phase 1)
             ["TerminalOutputFlushIntervalMs"] = ("INTEGER", AppConstants.MigrationDefaults.TerminalOutputFlushIntervalMs.ToString()),
             ["TerminalOutputMaxBatchSize"] = ("INTEGER", AppConstants.MigrationDefaults.TerminalOutputMaxBatchSize.ToString()),
@@ -216,6 +224,56 @@ await db.Database.ExecuteSqlRawAsync(
                 "ALTER TABLE Groups ADD COLUMN Color TEXT DEFAULT NULL");
             logger.Information("Added missing column Color to Groups table");
         }
+
+        await UpdateSchemaVersionAsync(connection, 1);
+        logger.Information("Schema version updated to 1");
+        } // end if (currentVersion < 1)
+
+        // Future migrations go here as: if (currentVersion < 2) { ... }
+    }
+
+    /// <summary>
+    /// Reads the current schema version from the SchemaVersion table.
+    /// Creates the table if it does not exist.
+    /// </summary>
+    private static async Task<int> GetSchemaVersionAsync(DbConnection connection)
+    {
+        // Create SchemaVersion table if it doesn't exist
+        await using (var cmd = connection.CreateCommand())
+        {
+            cmd.CommandText = "CREATE TABLE IF NOT EXISTS SchemaVersion (Version INTEGER NOT NULL DEFAULT 0)";
+            await cmd.ExecuteNonQueryAsync();
+        }
+
+        // Read current version
+        await using (var cmd = connection.CreateCommand())
+        {
+            cmd.CommandText = "SELECT Version FROM SchemaVersion LIMIT 1";
+            var result = await cmd.ExecuteScalarAsync();
+            if (result is null or DBNull)
+            {
+                // No row yet, insert default version 0
+                await using var insertCmd = connection.CreateCommand();
+                insertCmd.CommandText = "INSERT INTO SchemaVersion (Version) VALUES (0)";
+                await insertCmd.ExecuteNonQueryAsync();
+                return 0;
+            }
+            return Convert.ToInt32(result);
+        }
+    }
+
+    /// <summary>
+    /// Updates the schema version to the specified value.
+    /// </summary>
+    private static async Task UpdateSchemaVersionAsync(DbConnection connection, int version)
+    {
+        await using var cmd = connection.CreateCommand();
+        cmd.CommandText = "UPDATE SchemaVersion SET Version = @version";
+        var param = cmd.CreateParameter();
+        param.ParameterName = "@version";
+        param.Value = version;
+        cmd.Parameters.Add(param);
+        await cmd.ExecuteNonQueryAsync();
     }
 
     /// <summary>

src/SshManager.App/Infrastructure/DbMigrator.cs

@@ -58,10 +58,10 @@ public async Task<HostStatus> CheckHostAsync(Guid hostId, string hostname, int p
             var pingTask = TryPingAsync(hostname, ct);
             var tcpTask = TryTcpAsync(hostname, port, ct);
 
-            await Task.WhenAll(pingTask, tcpTask);
+            await Task.WhenAll(pingTask, tcpTask).ConfigureAwait(false);
 
-            var pingLatencyMs = pingTask.Result;
-            var tcpLatencyMs = tcpTask.Result;
+            var pingLatencyMs = await pingTask.ConfigureAwait(false);
+            var tcpLatencyMs = await tcpTask.ConfigureAwait(false);
 
             // Determine online status and build enhanced status
             var isOnline = pingLatencyMs.HasValue || tcpLatencyMs.HasValue;
@@ -137,7 +137,16 @@ public void ClearHosts()
         try
         {
             using var ping = new Ping();
-            var reply = await ping.SendPingAsync(hostname, PingTimeoutMs);
+            var pingTask = ping.SendPingAsync(hostname, PingTimeoutMs);
+            var cancelTask = Task.Delay(Timeout.Infinite, ct);
+            var completed = await Task.WhenAny(pingTask, cancelTask).ConfigureAwait(false);
+
+            if (completed == cancelTask)
+            {
+                ct.ThrowIfCancellationRequested();
+            }
+
+            var reply = await pingTask.ConfigureAwait(false);
             return reply.Status == IPStatus.Success
                 ? (int)reply.RoundtripTime
                 : null;