From 72b057cf199dca63ba9e009731e27333a515b8c7 Mon Sep 17 00:00:00 2001
From: tee <Thomas_BIRRELL@proton.me>
Date: Sat, 26 Sep 2026 19:11:51 -0400
Subject: [PATCH 1/2] fix: address high-severity code-review findings

Multi-agent review surfaced five HIGH findings; this fixes all of them.
All 1013 tests pass.

- OpenIvExecutor (H1): write to a temp sibling then atomic File.Move on
  overwrite, so a failed or interrupted copy never truncates an existing
  game file in place. Adds an overwrite-failure regression test proving the
  original survives and no .tmp is left behind.
- ChangeHistoryService (H4/H5): persist via JsonFileStore (logged, atomic
  temp-write) instead of a bare catch{} that silently lost the audit trail;
  guard the entry list with a lock and a Lazy singleton; Entries now returns
  a snapshot. No more swallowed saves or torn cross-thread reads.
- ProfileManager: log skipped corrupt profiles instead of dropping them
  silently, never seed stock defaults over unparseable user data, and write
  profiles atomically through JsonFileStore.
- JobQueue (H3): serialize all job-state access under a lock and hand the
  /jobs/{id} poller a detached snapshot, so it can never observe torn state
  (e.g. State=Completed before ResultJson is visible).
- LocalApi (H2): add ApiErrors.Problem helper and route 21 catch handlers
  through it, so exception text (absolute file paths) is logged server-side
  with a correlation id rather than leaked to the client.
---
 LSPDFRManager.LocalApi/ApiErrors.cs           | 21 ++++++
 .../Endpoints/BackupEndpoints.cs              |  4 +-
 .../Endpoints/BrowseEndpoints.cs              |  2 +-
 .../Endpoints/CleanupEndpoints.cs             |  6 +-
 .../Endpoints/CompatibilityEndpoints.cs       |  2 +-
 .../Endpoints/DiagnosticsEndpoints.cs         |  2 +-
 .../Endpoints/HistoryEndpoints.cs             |  2 +-
 .../Endpoints/LibraryEndpoints.cs             | 12 +--
 .../Endpoints/LogEndpoints.cs                 |  2 +-
 .../Endpoints/ProfileEndpoints.cs             |  8 +-
 .../Endpoints/SafeModeEndpoints.cs            |  2 +-
 LSPDFRManager.LocalApi/Services/JobQueue.cs   | 70 +++++++++++++-----
 .../Core/CarInstall/OpenIvExecutor.cs         | 27 ++++++-
 .../Services/ChangeHistoryService.cs          | 74 +++++++++----------
 .../OpenIvExecutorIntegrationTests.cs         | 41 ++++++++++
 Services/ProfileManager.cs                    | 20 +++--
 16 files changed, 208 insertions(+), 87 deletions(-)
 create mode 100644 LSPDFRManager.LocalApi/ApiErrors.cs

diff --git a/LSPDFRManager.LocalApi/ApiErrors.cs b/LSPDFRManager.LocalApi/ApiErrors.cs
new file mode 100644
index 0000000..0f87368
--- /dev/null
+++ b/LSPDFRManager.LocalApi/ApiErrors.cs
@@ -0,0 +1,21 @@
+using LSPDFRManager.Core;
+
+namespace LSPDFRManager.LocalApi;
+
+/// <summary>
+/// Centralized 500-level error responses. Logs the full exception server-side
+/// with a short correlation id and returns only a generic message to the
+/// client, so internal detail — absolute file paths, stack traces — never
+/// leaks over the API. Endpoints pass a human context string, not ex.Message.
+/// </summary>
+public static class ApiErrors
+{
+    public static IResult Problem(string context, Exception ex)
+    {
+        var correlationId = Guid.NewGuid().ToString("N")[..8];
+        AppLogger.Error($"[API_ERROR {correlationId}] {context}", ex);
+        return Results.Problem(
+            detail: $"{context}. Reference id: {correlationId} (see app.log for details).",
+            statusCode: StatusCodes.Status500InternalServerError);
+    }
+}
diff --git a/LSPDFRManager.LocalApi/Endpoints/BackupEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/BackupEndpoints.cs
index 9273f92..601ee79 100644
--- a/LSPDFRManager.LocalApi/Endpoints/BackupEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/BackupEndpoints.cs
@@ -36,7 +36,7 @@ public static void MapBackups(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to list backups: {ex.Message}");
+                return ApiErrors.Problem($"Failed to list backups", ex);
             }
         });
 
@@ -160,7 +160,7 @@ public static void MapBackups(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to delete backup: {ex.Message}");
+                return ApiErrors.Problem($"Failed to delete backup", ex);
             }
         });
 
diff --git a/LSPDFRManager.LocalApi/Endpoints/BrowseEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/BrowseEndpoints.cs
index 3dd9f12..f290c9f 100644
--- a/LSPDFRManager.LocalApi/Endpoints/BrowseEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/BrowseEndpoints.cs
@@ -53,7 +53,7 @@ public static void MapBrowse(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Browse proxy error: {ex.Message}");
+                return ApiErrors.Problem($"Browse proxy error", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/CleanupEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/CleanupEndpoints.cs
index ff0cdc2..a21f17f 100644
--- a/LSPDFRManager.LocalApi/Endpoints/CleanupEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/CleanupEndpoints.cs
@@ -23,7 +23,7 @@ public static void MapCleanup(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Cleanup scan failed: {ex.Message}");
+                return ApiErrors.Problem($"Cleanup scan failed", ex);
             }
         });
 
@@ -50,7 +50,7 @@ public static void MapCleanup(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Re-scan failed: {ex.Message}");
+                return ApiErrors.Problem($"Re-scan failed", ex);
             }
 
             var pathSet = req.RelativePaths.ToHashSet(StringComparer.OrdinalIgnoreCase);
@@ -82,7 +82,7 @@ public static void MapCleanup(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Cleanup apply failed: {ex.Message}");
+                return ApiErrors.Problem($"Cleanup apply failed", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/CompatibilityEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/CompatibilityEndpoints.cs
index b20944c..0971f6e 100644
--- a/LSPDFRManager.LocalApi/Endpoints/CompatibilityEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/CompatibilityEndpoints.cs
@@ -45,7 +45,7 @@ public static void MapCompatibility(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to detect component versions: {ex.Message}");
+                return ApiErrors.Problem($"Failed to detect component versions", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/DiagnosticsEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/DiagnosticsEndpoints.cs
index e729920..8d21646 100644
--- a/LSPDFRManager.LocalApi/Endpoints/DiagnosticsEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/DiagnosticsEndpoints.cs
@@ -50,7 +50,7 @@ public static void MapDiagnostics(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Diagnostics scan failed: {ex.Message}");
+                return ApiErrors.Problem($"Diagnostics scan failed", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/HistoryEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/HistoryEndpoints.cs
index be17567..879464e 100644
--- a/LSPDFRManager.LocalApi/Endpoints/HistoryEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/HistoryEndpoints.cs
@@ -37,7 +37,7 @@ public static void MapHistory(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to read change history: {ex.Message}");
+                return ApiErrors.Problem($"Failed to read change history", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/LibraryEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/LibraryEndpoints.cs
index ebc84e2..12aa9d5 100644
--- a/LSPDFRManager.LocalApi/Endpoints/LibraryEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/LibraryEndpoints.cs
@@ -39,7 +39,7 @@ public static void MapLibrary(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to read library: {ex.Message}");
+                return ApiErrors.Problem($"Failed to read library", ex);
             }
         });
 
@@ -57,7 +57,7 @@ public static void MapLibrary(this WebApplication app)
                 }
                 catch (Exception ex)
                 {
-                    return Results.Problem($"Failed to toggle mod: {ex.Message}");
+                    return ApiErrors.Problem($"Failed to toggle mod", ex);
                 }
             }
 
@@ -75,7 +75,7 @@ public static void MapLibrary(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to toggle mod: {ex.Message}");
+                return ApiErrors.Problem($"Failed to toggle mod", ex);
             }
             finally
             {
@@ -97,7 +97,7 @@ public static void MapLibrary(this WebApplication app)
                 }
                 catch (Exception ex)
                 {
-                    return Results.Problem($"Failed to update notes: {ex.Message}");
+                    return ApiErrors.Problem($"Failed to update notes", ex);
                 }
             }
 
@@ -115,7 +115,7 @@ public static void MapLibrary(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to update notes: {ex.Message}");
+                return ApiErrors.Problem($"Failed to update notes", ex);
             }
             finally
             {
@@ -148,7 +148,7 @@ public static void MapLibrary(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Sync failed: {ex.Message}");
+                return ApiErrors.Problem($"Sync failed", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/LogEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/LogEndpoints.cs
index 074584b..7bea9d2 100644
--- a/LSPDFRManager.LocalApi/Endpoints/LogEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/LogEndpoints.cs
@@ -58,7 +58,7 @@ public static void MapLogs(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to read log '{name}': {ex.Message}");
+                return ApiErrors.Problem($"Failed to read log '{name}'", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/ProfileEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/ProfileEndpoints.cs
index f15d132..9c966d7 100644
--- a/LSPDFRManager.LocalApi/Endpoints/ProfileEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/ProfileEndpoints.cs
@@ -25,7 +25,7 @@ public static void MapProfiles(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to read profiles: {ex.Message}");
+                return ApiErrors.Problem($"Failed to read profiles", ex);
             }
         });
 
@@ -46,7 +46,7 @@ public static void MapProfiles(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to create profile: {ex.Message}");
+                return ApiErrors.Problem($"Failed to create profile", ex);
             }
         });
 
@@ -75,7 +75,7 @@ public static void MapProfiles(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to update profile: {ex.Message}");
+                return ApiErrors.Problem($"Failed to update profile", ex);
             }
         });
 
@@ -98,7 +98,7 @@ public static void MapProfiles(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to delete profile: {ex.Message}");
+                return ApiErrors.Problem($"Failed to delete profile", ex);
             }
         });
     }
diff --git a/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs
index 4c9b23e..7aa42cc 100644
--- a/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs
@@ -46,7 +46,7 @@ public static void MapSafeMode(this WebApplication app)
             }
             catch (Exception ex)
             {
-                return Results.Problem($"Failed to build safe mode plan: {ex.Message}");
+                return ApiErrors.Problem($"Failed to build safe mode plan", ex);
             }
         });
 
diff --git a/LSPDFRManager.LocalApi/Services/JobQueue.cs b/LSPDFRManager.LocalApi/Services/JobQueue.cs
index a55d8c5..dc8026f 100644
--- a/LSPDFRManager.LocalApi/Services/JobQueue.cs
+++ b/LSPDFRManager.LocalApi/Services/JobQueue.cs
@@ -11,11 +11,25 @@ public sealed class JobEntry
     public string? ResultJson { get; set; }
     public DateTime StartedAt { get; init; } = DateTime.UtcNow;
     public DateTime? CompletedAt { get; set; }
+
+    // A detached copy handed to readers so they never observe a field being
+    // mutated by a background thread mid-read.
+    internal JobEntry Snapshot() => new()
+    {
+        JobId = JobId,
+        State = State,
+        ProgressPct = ProgressPct,
+        Error = Error,
+        ResultJson = ResultJson,
+        StartedAt = StartedAt,
+        CompletedAt = CompletedAt,
+    };
 }
 
 public sealed class JobQueue : IDisposable
 {
     private readonly ConcurrentDictionary<string, JobEntry> _jobs = new();
+    private readonly object _lock = new();
     private readonly Timer _pruner;
 
     public JobQueue()
@@ -27,52 +41,68 @@ public JobQueue()
     public string CreateJob()
     {
         var jobId = Guid.NewGuid().ToString("N");
-        _jobs[jobId] = new JobEntry { JobId = jobId };
+        lock (_lock)
+            _jobs[jobId] = new JobEntry { JobId = jobId };
         return jobId;
     }
 
     public void UpdateProgress(string jobId, int pct, string state)
     {
-        if (_jobs.TryGetValue(jobId, out var job))
+        lock (_lock)
         {
-            job.ProgressPct = pct;
-            job.State = state;
+            if (_jobs.TryGetValue(jobId, out var job))
+            {
+                job.ProgressPct = pct;
+                job.State = state;
+            }
         }
     }
 
     public void CompleteJob(string jobId, string? resultJson = null)
     {
-        if (_jobs.TryGetValue(jobId, out var job))
+        lock (_lock)
         {
-            job.State = "Completed";
-            job.ProgressPct = 100;
-            job.ResultJson = resultJson;
-            job.CompletedAt = DateTime.UtcNow;
+            if (_jobs.TryGetValue(jobId, out var job))
+            {
+                job.State = "Completed";
+                job.ProgressPct = 100;
+                job.ResultJson = resultJson;
+                job.CompletedAt = DateTime.UtcNow;
+            }
         }
     }
 
     public void FailJob(string jobId, string? error)
     {
-        if (_jobs.TryGetValue(jobId, out var job))
+        lock (_lock)
         {
-            job.State = "Failed";
-            job.Error = error;
-            job.CompletedAt = DateTime.UtcNow;
+            if (_jobs.TryGetValue(jobId, out var job))
+            {
+                job.State = "Failed";
+                job.Error = error;
+                job.CompletedAt = DateTime.UtcNow;
+            }
         }
     }
 
-    public JobEntry? GetJob(string jobId) =>
-        _jobs.TryGetValue(jobId, out var job) ? job : null;
+    public JobEntry? GetJob(string jobId)
+    {
+        lock (_lock)
+            return _jobs.TryGetValue(jobId, out var job) ? job.Snapshot() : null;
+    }
 
     private void Prune()
     {
         var cutoff = DateTime.UtcNow.AddMinutes(-10);
-        foreach (var kv in _jobs)
+        lock (_lock)
         {
-            var job = kv.Value;
-            var isTerminal = job.State is "Completed" or "Failed" or "Cancelled";
-            if (isTerminal && job.CompletedAt.HasValue && job.CompletedAt.Value < cutoff)
-                _jobs.TryRemove(kv.Key, out _);
+            foreach (var kv in _jobs)
+            {
+                var job = kv.Value;
+                var isTerminal = job.State is "Completed" or "Failed" or "Cancelled";
+                if (isTerminal && job.CompletedAt.HasValue && job.CompletedAt.Value < cutoff)
+                    _jobs.TryRemove(kv.Key, out _);
+            }
         }
     }
 
diff --git a/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs b/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs
index fc0940a..c5fccab 100644
--- a/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs
+++ b/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs
@@ -157,6 +157,19 @@ private static void DeleteBackupRoot(string backupRoot)
         }
     }
 
+    private static void TryDeleteTemp(string tempPath)
+    {
+        try
+        {
+            if (File.Exists(tempPath))
+                File.Delete(tempPath);
+        }
+        catch (Exception ex)
+        {
+            AppLogger.Warning($"[COPY_TEMP_CLEANUP] {Path.GetFileName(tempPath)} | {ex.Message}");
+        }
+    }
+
     private static int SelectBufferSize(long fileSize)
     {
         if (fileSize < 1_000_000)
@@ -194,15 +207,21 @@ private static async Task SafeCopyAsync(
 
         for (int attempt = 0; attempt < MaxRetries; attempt++)
         {
+            // Write to a temp sibling then commit with an atomic move, so an
+            // existing destination is never truncated in place — a crash or
+            // power-loss mid-copy leaves the original file intact for rollback.
+            var tempPath = destPath + "." + Guid.NewGuid().ToString("N") + ".tmp";
             try
             {
                 ct.ThrowIfCancellationRequested();
 
-                using (var destFile = File.Create(destPath))
+                using (var destFile = File.Create(tempPath))
                 {
                     await source.CopyToAsync(destFile, bufferSize, ct);
                 }
 
+                File.Move(tempPath, destPath, overwrite: true);
+
                 AppLogger.Info($"[COPY_OK] {fileName}");
                 return;
             }
@@ -213,6 +232,12 @@ private static async Task SafeCopyAsync(
                 backoff *= 2;
                 source.Seek(0, SeekOrigin.Begin);
             }
+            finally
+            {
+                // On success the temp was already moved (no-op); on any failure
+                // or retry, drop the partial temp so nothing leaks.
+                TryDeleteTemp(tempPath);
+            }
         }
 
         AppLogger.Error($"[COPY_FAILED] {fileName} | exhausted {MaxRetries} attempts");
diff --git a/LSPDFRManager.Shared/Services/ChangeHistoryService.cs b/LSPDFRManager.Shared/Services/ChangeHistoryService.cs
index c0840e1..9db53d8 100644
--- a/LSPDFRManager.Shared/Services/ChangeHistoryService.cs
+++ b/LSPDFRManager.Shared/Services/ChangeHistoryService.cs
@@ -4,44 +4,50 @@ namespace LSPDFRManager.Services;
 
 public class ChangeHistoryService
 {
-    private static ChangeHistoryService? _instance;
-    public static ChangeHistoryService Instance => _instance ??= new();
+    private static readonly Lazy<ChangeHistoryService> _instance = new(() => new ChangeHistoryService());
+    public static ChangeHistoryService Instance => _instance.Value;
 
-    private List<ChangeHistoryEntry> _entries = [];
+    private readonly JsonFileStore<List<ChangeHistoryEntry>> _store = new(AppDataPaths.ChangeHistoryFile);
+    private readonly object _lock = new();
+    private List<ChangeHistoryEntry> _entries;
 
-    internal ChangeHistoryService() => Load();
+    internal ChangeHistoryService() => _entries = _store.LoadOrDefault(() => []);
 
-    public IReadOnlyList<ChangeHistoryEntry> Entries => _entries;
+    // Returns a snapshot: callers iterate a stable copy, never the live list.
+    public IReadOnlyList<ChangeHistoryEntry> Entries
+    {
+        get { lock (_lock) return _entries.ToList(); }
+    }
 
     public void Load()
     {
-        var path = AppDataPaths.ChangeHistoryFile;
-        if (!File.Exists(path)) return;
-        try
-        {
-            var json = File.ReadAllText(path);
-            _entries = System.Text.Json.JsonSerializer.Deserialize<List<ChangeHistoryEntry>>(json) ?? [];
-        }
-        catch { _entries = []; }
+        var loaded = _store.LoadOrDefault(() => []);
+        lock (_lock) _entries = loaded;
     }
 
     public void Record(ChangeHistoryAction action, string description, string? affectedFile = null, string? detail = null)
     {
-        _entries.Insert(0, new ChangeHistoryEntry
+        List<ChangeHistoryEntry> snapshot;
+        lock (_lock)
         {
-            Action = action,
-            Description = description,
-            AffectedFile = affectedFile,
-            Detail = detail,
-        });
+            _entries.Insert(0, new ChangeHistoryEntry
+            {
+                Action = action,
+                Description = description,
+                AffectedFile = affectedFile,
+                Detail = detail,
+            });
 
-        if (_entries.Count > 1000) _entries = _entries.Take(1000).ToList();
-        Save();
+            if (_entries.Count > 1000) _entries = _entries.Take(1000).ToList();
+            snapshot = _entries.ToList();
+        }
+        _store.Save(snapshot);
     }
 
     public List<ChangeHistoryEntry> Filter(ChangeHistoryAction? action = null, DateTime? since = null, string? search = null)
     {
-        IEnumerable<ChangeHistoryEntry> q = _entries;
+        IEnumerable<ChangeHistoryEntry> q;
+        lock (_lock) q = _entries.ToList();
         if (action.HasValue) q = q.Where(e => e.Action == action.Value);
         if (since.HasValue) q = q.Where(e => e.OccurredAt >= since.Value);
         if (!string.IsNullOrWhiteSpace(search))
@@ -52,15 +58,18 @@ public List<ChangeHistoryEntry> Filter(ChangeHistoryAction? action = null, DateT
 
     public void Clear()
     {
-        _entries.Clear();
-        Save();
+        lock (_lock) _entries.Clear();
+        _store.Save([]);
     }
 
     public async Task ExportAsync(string outputPath, bool asJson)
     {
+        List<ChangeHistoryEntry> entries;
+        lock (_lock) entries = _entries.ToList();
+
         if (asJson)
         {
-            var sanitized = _entries.Select(e => new
+            var sanitized = entries.Select(e => new
             {
                 e.Id, e.Action, e.Description, e.OccurredAt, e.Detail,
                 AffectedFile = SanitizePath(e.AffectedFile),
@@ -71,7 +80,7 @@ public async Task ExportAsync(string outputPath, bool asJson)
         }
         else
         {
-            var lines = _entries.Select(e =>
+            var lines = entries.Select(e =>
             {
                 var line = $"[{e.OccurredAt:yyyy-MM-dd HH:mm:ss}] [{e.Action}] {e.Description}";
                 var af   = SanitizePath(e.AffectedFile);
@@ -92,17 +101,4 @@ private static string SanitizePath(string? path)
             return "%USERPROFILE%" + path[home.Length..];
         return path;
     }
-
-    private void Save()
-    {
-        try
-        {
-            var path = AppDataPaths.ChangeHistoryFile;
-            var dir = Path.GetDirectoryName(path);
-            if (dir is not null) Directory.CreateDirectory(dir);
-            var json = System.Text.Json.JsonSerializer.Serialize(_entries, new System.Text.Json.JsonSerializerOptions { WriteIndented = true });
-            File.WriteAllText(path, json);
-        }
-        catch { }
-    }
 }
diff --git a/LSPDFRManager.Tests/OpenIvExecutorIntegrationTests.cs b/LSPDFRManager.Tests/OpenIvExecutorIntegrationTests.cs
index 1e46011..22edd47 100644
--- a/LSPDFRManager.Tests/OpenIvExecutorIntegrationTests.cs
+++ b/LSPDFRManager.Tests/OpenIvExecutorIntegrationTests.cs
@@ -181,6 +181,47 @@ public async Task Executor_TraversalDestination_FailsWithoutWritingOutsideRoot()
         Assert.False(File.Exists(Path.Combine(_tempRoot, "escape.meta")));
     }
 
+    // ── Test 7: Overwrite failure preserves the original file ────────────
+    // Guards the temp-then-move write discipline: a mid-copy failure while
+    // overwriting an existing file must never truncate the original, and must
+    // leave no partial .tmp files behind.
+
+    [Fact]
+    public async Task Executor_OverwriteFailsMidCopy_OriginalPreservedAndNoTempLeftover()
+    {
+        var destRel = @"mods\existing.dll";
+        var destPath = Path.Combine(_tempRoot, destRel);
+        Directory.CreateDirectory(Path.GetDirectoryName(destPath)!);
+        var originalContent = new byte[] { 100, 101, 102, 103, 104 };
+        File.WriteAllBytes(destPath, originalContent);
+
+        var archive = new FakeArchive(new[]
+        {
+            new FakeArchiveEntry("existing.dll",
+                () => new ThrowingStream(new byte[] { 1, 2, 3, 4, 5, 6 }, failAfter: 2),
+                size: 6)
+        });
+        var plan = new OpenIvInstallPlan
+        {
+            Type = CarInstallType.ReplaceVehicle,
+            TargetDlcName = "overwrite_mod",
+            Operations = new()
+            {
+                new() { SourcePath = "existing.dll", DestinationPath = destRel, Overwrite = true }
+            },
+            XmlPatches = new()
+        };
+
+        var result = await _executor.ExecuteAsync(plan, archive, _tempRoot);
+
+        Assert.False(result.Success);
+        Assert.True(File.Exists(destPath), "original file must survive a failed overwrite");
+        Assert.Equal(originalContent, File.ReadAllBytes(destPath));
+
+        var leftoverTemps = Directory.GetFiles(Path.GetDirectoryName(destPath)!, "*.tmp");
+        Assert.Empty(leftoverTemps);
+    }
+
     private void SetupDlcListXml(string targetRoot)
     {
         var dlclistDir = Path.Combine(targetRoot, @"mods\update\update.rpf\common\data");
diff --git a/Services/ProfileManager.cs b/Services/ProfileManager.cs
index 06fc86b..39447b6 100644
--- a/Services/ProfileManager.cs
+++ b/Services/ProfileManager.cs
@@ -18,7 +18,8 @@ public void Load()
         var dir = AppDataPaths.ProfilesDirectory;
         Directory.CreateDirectory(dir);
 
-        foreach (var file in Directory.EnumerateFiles(dir, "*.json"))
+        var files = Directory.EnumerateFiles(dir, "*.json").ToList();
+        foreach (var file in files)
         {
             try
             {
@@ -26,10 +27,16 @@ public void Load()
                 var profile = System.Text.Json.JsonSerializer.Deserialize<ModProfile>(json);
                 if (profile is not null) _profiles.Add(profile);
             }
-            catch { }
+            catch (Exception ex)
+            {
+                AppLogger.Warning($"[ProfileManager] Skipped corrupt profile '{Path.GetFileName(file)}': {ex.Message}");
+            }
         }
 
-        if (_profiles.Count == 0)
+        // Seed stock defaults only when the profiles dir is genuinely empty —
+        // never when files existed but failed to parse, which would silently
+        // replace the user's real profiles with stock ones.
+        if (_profiles.Count == 0 && files.Count == 0)
             SeedDefaults();
     }
 
@@ -200,9 +207,10 @@ private void SaveAll()
 
     private void SaveProfile(ModProfile profile)
     {
-        var path = ProfilePath(profile);
-        var json = System.Text.Json.JsonSerializer.Serialize(profile, new System.Text.Json.JsonSerializerOptions { WriteIndented = true });
-        File.WriteAllText(path, json);
+        // JsonFileStore writes to a temp file then atomically replaces, so a
+        // crash mid-write never truncates an existing profile, and I/O failures
+        // are logged rather than thrown as a hard failure out of Create/Apply.
+        new JsonFileStore<ModProfile>(ProfilePath(profile)).Save(profile);
     }
 
     private static List<ProfileEntry> SnapshotCurrentLibrary()

From 0440c19a1d3a9d73bdf476fcccbf5f85579362b0 Mon Sep 17 00:00:00 2001
From: tee <Thomas_BIRRELL@proton.me>
Date: Sat, 26 Sep 2026 19:18:35 -0400
Subject: [PATCH 2/2] fix: medium-severity hardening + release v3.7.24

Second review pass: closes the remaining testable MEDIUM findings and cuts
release 3.7.24. All 1021 tests pass (+8 new).

- SafeModeEndpoints: contain restore to the GTA V root via new
  PathContainment.IsWithin helper, so a tampered/corrupt safe-mode manifest
  cannot move files to/from arbitrary locations. Adds PathContainmentTests
  (inside/sibling-prefix/traversal/null cases).
- OpenIvExecutor: build a one-time key->entry archive lookup instead of
  re-enumerating per operation (O(n^2) -> O(n)); also avoids re-opening
  entries out of order on non-seekable streams.
- RestorePointService: Lazy singleton + lock-guarded snapshot reads/writes,
  closing the torn-read race between UI and background saves.
- Release 3.7.24: bump Version/AssemblyVersion/FileVersion in both csproj,
  update version assertions (VersionAndBrowseGuardTests, SetupWizardTests),
  add RELEASE_v3.7.24.md and a new CHANGELOG.md.
---
 CHANGELOG.md                                  | 33 ++++++++++++
 .../Endpoints/SafeModeEndpoints.cs            |  3 ++
 .../Core/CarInstall/OpenIvExecutor.cs         | 12 +++--
 .../LSPDFRManager.Shared.csproj               |  6 +--
 .../Services/PathContainment.cs               | 27 ++++++++++
 .../Services/RestorePointService.cs           | 41 ++++++++++-----
 LSPDFRManager.Tests/PathContainmentTests.cs   | 47 +++++++++++++++++
 LSPDFRManager.Tests/SetupWizardTests.cs       |  2 +-
 .../VersionAndBrowseGuardTests.cs             |  4 +-
 LSPDFRManager.csproj                          |  6 +--
 RELEASE_v3.7.24.md                            | 51 +++++++++++++++++++
 11 files changed, 207 insertions(+), 25 deletions(-)
 create mode 100644 CHANGELOG.md
 create mode 100644 LSPDFRManager.Shared/Services/PathContainment.cs
 create mode 100644 LSPDFRManager.Tests/PathContainmentTests.cs
 create mode 100644 RELEASE_v3.7.24.md

diff --git a/CHANGELOG.md b/CHANGELOG.md
new file mode 100644
index 0000000..b7b80c8
--- /dev/null
+++ b/CHANGELOG.md
@@ -0,0 +1,33 @@
+# Changelog
+
+All notable changes to this project are documented here. This project follows
+[Semantic Versioning](https://semver.org/). Per-release detail lives in the
+`RELEASE_v*.md` files; this file is the running summary.
+
+## [3.7.24] — 2026-09-26
+
+### Fixed
+- **Install integrity:** `OpenIvExecutor` writes to a temp file and commits with
+  an atomic move, so a failed/interrupted overwrite never truncates an existing
+  game file in place.
+- **Change history:** persisted via atomic `JsonFileStore` and lock-guarded;
+  saves are no longer silently swallowed on I/O error.
+- **Profiles:** corrupt profile files are logged and skipped instead of dropped,
+  stock defaults are never seeded over unparseable user data, and writes are
+  atomic.
+- **Restore points:** lock-guarded snapshot reads close a torn-read race.
+- **Job queue:** all job-state access is serialized and readers get a snapshot,
+  removing a race where `/jobs/{id}` could return torn state.
+
+### Security
+- **Safe-mode restore** verifies each manifest path resolves inside the GTA V
+  root before moving it, containing tampered/corrupt manifests.
+- **Local API** error responses route through `ApiErrors.Problem`, logging the
+  exception server-side with a correlation id instead of leaking absolute file
+  paths to the client.
+
+### Performance
+- `OpenIvExecutor` builds a one-time archive entry lookup (O(n²) → O(n)).
+
+### Tests
+- 1021 passing (+8): overwrite-failure integrity, path containment.
diff --git a/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs b/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs
index 7aa42cc..00bdbf5 100644
--- a/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs
+++ b/LSPDFRManager.LocalApi/Endpoints/SafeModeEndpoints.cs
@@ -116,6 +116,9 @@ public static void MapSafeMode(this WebApplication app)
                 foreach (var disabledPath in manifest.DisabledPaths)
                 {
                     if (!disabledPath.EndsWith(".disabled", StringComparison.OrdinalIgnoreCase)) continue;
+                    // Contain the move to the GTA root: a tampered/corrupt manifest
+                    // must never move files to or from arbitrary locations.
+                    if (!PathContainment.IsWithin(gtaPath, disabledPath)) continue;
                     if (!File.Exists(disabledPath)) continue;
                     var original = disabledPath[..^".disabled".Length];
                     if (!File.Exists(original))
diff --git a/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs b/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs
index c5fccab..41c70bd 100644
--- a/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs
+++ b/LSPDFRManager.Shared/Core/CarInstall/OpenIvExecutor.cs
@@ -49,15 +49,19 @@ public async Task<InstallResult> ExecuteAsync(
 
         try
         {
+            // Build a one-time key→entry lookup so we don't re-enumerate the
+            // archive per operation (O(n²)) and don't risk re-opening entries
+            // out of order on non-seekable archive streams.
+            var entriesByKey = new Dictionary<string, IArchiveEntry>(StringComparer.Ordinal);
+            foreach (var e in archive.Entries)
+                entriesByKey.TryAdd(e.Key, e);
+
             // 1. Extract files from archive
             foreach (var operation in plan.Operations)
             {
                 ct.ThrowIfCancellationRequested();
 
-                var sourceEntry = archive.Entries
-                    .FirstOrDefault(e => e.Key == operation.SourcePath);
-
-                if (sourceEntry is null)
+                if (!entriesByKey.TryGetValue(operation.SourcePath, out var sourceEntry))
                     throw new InvalidOperationException(
                         $"Archive entry not found: {operation.SourcePath}");
 
diff --git a/LSPDFRManager.Shared/LSPDFRManager.Shared.csproj b/LSPDFRManager.Shared/LSPDFRManager.Shared.csproj
index 41a7831..895c171 100644
--- a/LSPDFRManager.Shared/LSPDFRManager.Shared.csproj
+++ b/LSPDFRManager.Shared/LSPDFRManager.Shared.csproj
@@ -5,9 +5,9 @@
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>
     <RootNamespace>LSPDFRManager</RootNamespace>
-    <Version>3.7.23</Version>
-    <AssemblyVersion>3.7.23.0</AssemblyVersion>
-    <FileVersion>3.7.23.0</FileVersion>
+    <Version>3.7.24</Version>
+    <AssemblyVersion>3.7.24.0</AssemblyVersion>
+    <FileVersion>3.7.24.0</FileVersion>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/LSPDFRManager.Shared/Services/PathContainment.cs b/LSPDFRManager.Shared/Services/PathContainment.cs
new file mode 100644
index 0000000..a19cb1f
--- /dev/null
+++ b/LSPDFRManager.Shared/Services/PathContainment.cs
@@ -0,0 +1,27 @@
+namespace LSPDFRManager.Services;
+
+/// <summary>
+/// Verifies that an absolute path resolves to a location inside a root
+/// directory. Uses a separator-terminated root so a sibling such as
+/// <c>C:\GTAV_evil</c> does not match root <c>C:\GTAV</c> by string prefix.
+/// Use this to contain operations that consume paths from persisted state
+/// (e.g. a safe-mode manifest) which a user or corruption could tamper with.
+/// </summary>
+public static class PathContainment
+{
+    public static bool IsWithin(string root, string candidate)
+    {
+        if (string.IsNullOrWhiteSpace(root) || string.IsNullOrWhiteSpace(candidate))
+            return false;
+
+        var normalizedRoot = Path.GetFullPath(root)
+            .TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
+            + Path.DirectorySeparatorChar;
+
+        string fullCandidate;
+        try { fullCandidate = Path.GetFullPath(candidate); }
+        catch { return false; }
+
+        return fullCandidate.StartsWith(normalizedRoot, StringComparison.OrdinalIgnoreCase);
+    }
+}
diff --git a/LSPDFRManager.Shared/Services/RestorePointService.cs b/LSPDFRManager.Shared/Services/RestorePointService.cs
index d16da9e..607fc93 100644
--- a/LSPDFRManager.Shared/Services/RestorePointService.cs
+++ b/LSPDFRManager.Shared/Services/RestorePointService.cs
@@ -5,11 +5,17 @@ namespace LSPDFRManager.Services;
 
 public class RestorePointService
 {
-    private static RestorePointService? _instance;
-    public static RestorePointService Instance => _instance ??= new();
+    private static readonly Lazy<RestorePointService> _instance = new(() => new RestorePointService());
+    public static RestorePointService Instance => _instance.Value;
 
+    private readonly object _lock = new();
     private List<RestorePoint> _points = [];
-    public IReadOnlyList<RestorePoint> Points => _points;
+
+    // Snapshot: callers iterate a stable copy, never the live list.
+    public IReadOnlyList<RestorePoint> Points
+    {
+        get { lock (_lock) return _points.ToList(); }
+    }
 
     public RestorePointService() => Load();
 
@@ -20,17 +26,23 @@ public void Load()
         try
         {
             var json = File.ReadAllText(indexPath);
-            _points = System.Text.Json.JsonSerializer.Deserialize<List<RestorePoint>>(json) ?? [];
+            var loaded = System.Text.Json.JsonSerializer.Deserialize<List<RestorePoint>>(json) ?? [];
+            lock (_lock) _points = loaded;
         }
-        catch { _points = []; }
+        catch { lock (_lock) _points = []; }
     }
 
     public virtual async Task SaveAsync(RestorePoint point, CancellationToken cancellationToken = default)
     {
         cancellationToken.ThrowIfCancellationRequested();
-        _points.Insert(0, point);
-        if (_points.Count > 50) _points = _points.Take(50).ToList();
-        await PersistIndexAsync(cancellationToken);
+        List<RestorePoint> snapshot;
+        lock (_lock)
+        {
+            _points.Insert(0, point);
+            if (_points.Count > 50) _points = _points.Take(50).ToList();
+            snapshot = _points.ToList();
+        }
+        await PersistIndexAsync(snapshot, cancellationToken);
         ChangeHistoryService.Instance.Record(ChangeHistoryAction.RestorePointCreated, $"Restore point created: {point.OperationName}", detail: point.Id);
     }
 
@@ -77,15 +89,20 @@ public async Task RestoreAsync(RestorePoint point, IProgress<string>? progress =
 
     public async Task DeleteAsync(RestorePoint point)
     {
-        _points.Remove(point);
-        await PersistIndexAsync();
+        List<RestorePoint> snapshot;
+        lock (_lock)
+        {
+            _points.Remove(point);
+            snapshot = _points.ToList();
+        }
+        await PersistIndexAsync(snapshot);
     }
 
-    private async Task PersistIndexAsync(CancellationToken cancellationToken = default)
+    private static async Task PersistIndexAsync(List<RestorePoint> points, CancellationToken cancellationToken = default)
     {
         var dir = AppDataPaths.RestorePointsDirectory;
         Directory.CreateDirectory(dir);
-        var json = System.Text.Json.JsonSerializer.Serialize(_points, new System.Text.Json.JsonSerializerOptions { WriteIndented = true });
+        var json = System.Text.Json.JsonSerializer.Serialize(points, new System.Text.Json.JsonSerializerOptions { WriteIndented = true });
         await File.WriteAllTextAsync(AppDataPaths.RestorePointsIndex, json, cancellationToken);
     }
 }
diff --git a/LSPDFRManager.Tests/PathContainmentTests.cs b/LSPDFRManager.Tests/PathContainmentTests.cs
new file mode 100644
index 0000000..3ba3f53
--- /dev/null
+++ b/LSPDFRManager.Tests/PathContainmentTests.cs
@@ -0,0 +1,47 @@
+using LSPDFRManager.Services;
+using Xunit;
+
+namespace LSPDFRManager.Tests;
+
+public class PathContainmentTests
+{
+    [Fact]
+    public void IsWithin_FileInsideRoot_ReturnsTrue()
+    {
+        Assert.True(PathContainment.IsWithin(@"C:\GTAV", @"C:\GTAV\plugins\lspdfr\x.dll"));
+    }
+
+    [Fact]
+    public void IsWithin_RootItself_TrailingSeparatorTolerated()
+    {
+        Assert.True(PathContainment.IsWithin(@"C:\GTAV\", @"C:\GTAV\mods\y.dll"));
+    }
+
+    [Fact]
+    public void IsWithin_SiblingPrefixDirectory_ReturnsFalse()
+    {
+        // C:\GTAV_evil must not match root C:\GTAV by string prefix.
+        Assert.False(PathContainment.IsWithin(@"C:\GTAV", @"C:\GTAV_evil\payload.dll"));
+    }
+
+    [Fact]
+    public void IsWithin_TraversalEscapingRoot_ReturnsFalse()
+    {
+        Assert.False(PathContainment.IsWithin(@"C:\GTAV", @"C:\GTAV\..\Windows\System32\evil.dll"));
+    }
+
+    [Fact]
+    public void IsWithin_CompletelyDifferentRoot_ReturnsFalse()
+    {
+        Assert.False(PathContainment.IsWithin(@"C:\GTAV", @"D:\Other\thing.dll"));
+    }
+
+    [Theory]
+    [InlineData("", @"C:\GTAV\x.dll")]
+    [InlineData(@"C:\GTAV", "")]
+    [InlineData(null, @"C:\GTAV\x.dll")]
+    public void IsWithin_NullOrEmptyInput_ReturnsFalse(string? root, string? candidate)
+    {
+        Assert.False(PathContainment.IsWithin(root!, candidate!));
+    }
+}
diff --git a/LSPDFRManager.Tests/SetupWizardTests.cs b/LSPDFRManager.Tests/SetupWizardTests.cs
index fc2eb0b..0642665 100644
--- a/LSPDFRManager.Tests/SetupWizardTests.cs
+++ b/LSPDFRManager.Tests/SetupWizardTests.cs
@@ -75,7 +75,7 @@ public async Task UpdateCheck_ReturnsValidResult()
         var result = await new UpdateCheckService().CheckAsync();
 
         Assert.NotNull(result);
-        Assert.Equal("3.7.23", result.CurrentVersion);
+        Assert.Equal("3.7.24", result.CurrentVersion);
     }
 
     [Fact]
diff --git a/LSPDFRManager.Tests/VersionAndBrowseGuardTests.cs b/LSPDFRManager.Tests/VersionAndBrowseGuardTests.cs
index 0c87b41..89bf6dd 100644
--- a/LSPDFRManager.Tests/VersionAndBrowseGuardTests.cs
+++ b/LSPDFRManager.Tests/VersionAndBrowseGuardTests.cs
@@ -6,11 +6,11 @@ namespace LSPDFRManager.Tests;
 public class VersionAndBrowseGuardTests
 {
     [Fact]
-    public void AssemblyVersion_Is_3_7_23_0()
+    public void AssemblyVersion_Is_3_7_24_0()
     {
         var version = typeof(MainViewModel).Assembly.GetName().Version;
         Assert.NotNull(version);
-        Assert.Equal(new Version(3, 7, 23, 0), version);
+        Assert.Equal(new Version(3, 7, 24, 0), version);
     }
 
     [Fact]
diff --git a/LSPDFRManager.csproj b/LSPDFRManager.csproj
index 3e571c6..616edf7 100644
--- a/LSPDFRManager.csproj
+++ b/LSPDFRManager.csproj
@@ -11,9 +11,9 @@
     <Description>Desktop mod manager for GTA V LSPDFR installations.</Description>
     <Authors>rolling-codes</Authors>
     <RepositoryUrl>https://github.com/rolling-codes/LSPDFRManager</RepositoryUrl>
-    <Version>3.7.23</Version>
-    <AssemblyVersion>3.7.23.0</AssemblyVersion>
-    <FileVersion>3.7.23.0</FileVersion>
+    <Version>3.7.24</Version>
+    <AssemblyVersion>3.7.24.0</AssemblyVersion>
+    <FileVersion>3.7.24.0</FileVersion>
     <ApplicationManifest>app.manifest</ApplicationManifest>
   </PropertyGroup>
 
diff --git a/RELEASE_v3.7.24.md b/RELEASE_v3.7.24.md
new file mode 100644
index 0000000..f3c185d
--- /dev/null
+++ b/RELEASE_v3.7.24.md
@@ -0,0 +1,51 @@
+# Release v3.7.24 — Reliability & Hardening
+
+## Highlights
+
+A focused reliability release from a full multi-agent code review. No new
+features — this hardens the install/rollback path, closes silent data-loss
+windows in persistence, removes a concurrency race in the job queue, and stops
+the local API from leaking internal file paths in error responses.
+
+## Install integrity
+
+- **OIV executor no longer truncates files in place.** `OpenIvExecutor` now
+  writes each extracted file to a temp sibling and commits it with an atomic
+  `File.Move`. Previously an overwrite was truncated before the new bytes
+  landed, so a crash or power-loss mid-copy could leave a valid game file
+  destroyed. The original is now intact until the move succeeds.
+- **Faster, safer archive lookups.** The executor builds a one-time key→entry
+  map instead of re-enumerating the archive per operation (O(n²) → O(n)), which
+  also avoids re-opening entries out of order on non-seekable archive streams.
+
+## Data-loss fixes
+
+- **Change history** is now persisted through the atomic, logged `JsonFileStore`
+  instead of a bare `catch {}` that silently discarded the audit trail on any
+  I/O error. Entry-list access is guarded by a lock.
+- **Profiles** no longer vanish silently: a corrupt profile file is logged and
+  skipped, and stock defaults are never seeded over profiles that merely failed
+  to parse (which previously replaced the user's real profiles). Profiles are
+  written atomically.
+- **Restore points** access is now lock-guarded with snapshot reads, closing a
+  torn-read race between the UI and background saves.
+
+## Security & concurrency
+
+- **Job queue race fixed.** `/jobs/{id}` pollers can no longer observe torn job
+  state (e.g. `State=Completed` before the result is visible); all job-state
+  access is serialized and readers receive a detached snapshot.
+- **Safe-mode restore is contained.** The restore endpoint now verifies each
+  path in the safe-mode manifest resolves inside the configured GTA V root
+  before moving it, so a tampered or corrupt manifest cannot move files to or
+  from arbitrary locations.
+- **API errors no longer leak paths.** The local API's 500-level handlers route
+  through a new `ApiErrors.Problem` helper that logs the full exception
+  server-side with a correlation id and returns only a generic message —
+  absolute `%APPDATA%`/GTA paths are no longer exposed to the client.
+
+## Verification
+
+- 1021/1021 tests passing (8 new tests: overwrite-failure integrity, path
+  containment).
+- Clean build.