Reliability & hardening from multi-agent review (v3.7.24)

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.

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);
+    }
+}

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);
             }
         });
 

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);
             }
         });
     }

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);
             }
         });
     }

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);
             }
         });
     }

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);
             }
         });
     }

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);
             }
         });
     }

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);
             }
         });
     }

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);
             }
         });
     }

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);
             }
         });
     }

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);
             }
         });
 
@@ -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))

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 _);
+            }
         }
     }