From 9065199d91f7fcee0ff78c7c3834b553becaf9e0 Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Mon, 8 Jun 2026 14:43:11 +0300 Subject: [PATCH] fix: restore-point false success, updater asset match, WU COM leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2d of the audit — three correctness/safety fixes around update paths: - PerformanceService.CreateRestorePointAsync returned true whenever the call did not throw, but Checkpoint-Computer fails non-terminating (e.g. once-per-24h rate limit), so silent failures were reported as success. It now forces a terminating error and only returns true on an explicit success sentinel. - UpdateService matched a fixed 'SysManager.exe' asset that no release publishes (real name: SysManager-v.exe), so AssetUrl/AssetSize were always null. Replaced with IsMainExeAsset (versioned exe, excludes the .sha256 companion). - WindowsUpdateService.ScanAsync released COM objects only on the happy path, leaking them when cancellation/mapping threw mid-scan. Releases moved to finally. Tests: UpdateServiceAssetMatchTests (unit) + updated IntegrationTests Constants test now asserts IsMainExeAsset instead of the removed AssetName const. Full solution regression build clean across all four projects. --- CHANGELOG.md | 7 +++ .../UpdateServiceTests.cs | 10 +++- .../UpdateServiceAssetMatchTests.cs | 34 ++++++++++++ .../SysManager/Services/PerformanceService.cs | 20 +++++-- .../SysManager/Services/UpdateService.cs | 15 ++++-- .../Services/WindowsUpdateService.cs | 54 +++++++++++-------- SysManager/SysManager/SysManager.csproj | 6 +-- 7 files changed, 112 insertions(+), 34 deletions(-) create mode 100644 SysManager/SysManager.Tests/UpdateServiceAssetMatchTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index e541cde..df2cb8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [1.20.2] - 2026-06-08 + +### Fixed +- **Restore point creation no longer reports false success.** `CreateRestorePointAsync` returned `true` whenever the PowerShell call didn't throw, but `Checkpoint-Computer` fails *non-terminating* in common cases (notably the once-per-24h rate limit), so failures were reported as success — undermining the "everything is reversible" guarantee. It now forces the error to terminate and only returns `true` when an explicit success sentinel is emitted. +- **In-app updater can now find its download asset.** The release-asset matcher looked for a fixed `SysManager.exe`, but releases publish `SysManager-v.exe`, so `AssetUrl`/`AssetSize` were always null. Replaced with `IsMainExeAsset`, which matches the versioned executable and excludes the `.sha256` companion. +- **Windows Update scan no longer leaks COM objects on failure.** `WindowsUpdateService.ScanAsync` released its COM objects only on the success path, so a cancellation or mapping error mid-scan leaked them. The releases now run in a `finally` block. + ## [1.20.1] - 2026-06-08 ### Fixed diff --git a/SysManager/SysManager.IntegrationTests/UpdateServiceTests.cs b/SysManager/SysManager.IntegrationTests/UpdateServiceTests.cs index c2bc3b3..b3cad4b 100644 --- a/SysManager/SysManager.IntegrationTests/UpdateServiceTests.cs +++ b/SysManager/SysManager.IntegrationTests/UpdateServiceTests.cs @@ -80,7 +80,15 @@ public void Constants_AreSet() { Assert.Equal("laurentiu021", UpdateService.Owner); Assert.Equal("SystemManager", UpdateService.Repo); - Assert.Equal("SysManager.exe", UpdateService.AssetName); + } + + [Fact] + public void IsMainExeAsset_MatchesVersionedReleaseExe() + { + // Release assets are SysManager-v.exe, not a fixed SysManager.exe. + Assert.True(UpdateService.IsMainExeAsset("SysManager-v1.20.1.exe")); + Assert.False(UpdateService.IsMainExeAsset("SysManager-v1.20.1.exe.sha256")); + Assert.False(UpdateService.IsMainExeAsset("SysManager.exe")); } [Fact] diff --git a/SysManager/SysManager.Tests/UpdateServiceAssetMatchTests.cs b/SysManager/SysManager.Tests/UpdateServiceAssetMatchTests.cs new file mode 100644 index 0000000..82bbed1 --- /dev/null +++ b/SysManager/SysManager.Tests/UpdateServiceAssetMatchTests.cs @@ -0,0 +1,34 @@ +// SysManager · UpdateServiceAssetMatchTests +// Author: laurentiu021 · https://github.com/laurentiu021/SystemManager +// License: MIT + +using SysManager.Services; + +namespace SysManager.Tests; + +/// +/// Regression: the release asset matcher must accept the real versioned exe name +/// (SysManager-v<version>.exe) and reject the .sha256 companion. Previously it +/// looked for a fixed "SysManager.exe" that no release ever published, so the +/// in-app updater could never resolve its download asset. +/// +public class UpdateServiceAssetMatchTests +{ + [Theory] + [InlineData("SysManager-v1.20.1.exe")] + [InlineData("SysManager-v1.7.0.exe")] + [InlineData("SysManager-v2.0.0.exe")] + [InlineData("sysmanager-v1.20.1.EXE")] // case-insensitive + public void IsMainExeAsset_AcceptsVersionedExe(string name) + => Assert.True(UpdateService.IsMainExeAsset(name)); + + [Theory] + [InlineData("SysManager-v1.20.1.exe.sha256")] // checksum companion + [InlineData("SysManager.exe")] // the old fixed name no release uses + [InlineData("something-else.exe")] + [InlineData("SysManager-v1.20.1.zip")] + [InlineData("")] + [InlineData(null)] + public void IsMainExeAsset_RejectsNonMatching(string? name) + => Assert.False(UpdateService.IsMainExeAsset(name)); +} diff --git a/SysManager/SysManager/Services/PerformanceService.cs b/SysManager/SysManager/Services/PerformanceService.cs index ab47a8f..22e428c 100644 --- a/SysManager/SysManager/Services/PerformanceService.cs +++ b/SysManager/SysManager/Services/PerformanceService.cs @@ -529,11 +529,21 @@ public async Task CreateRestorePointAsync(string description, Cancellation // directly in the script body — with single-quote escaping to avoid // injection via the user-supplied string. var safeDesc = (description ?? "SysManager Restore Point").Replace("'", "''"); - var script = $"Checkpoint-Computer -Description '{safeDesc}' -RestorePointType 'MODIFY_SETTINGS'"; - await _ps.RunAsync(script, null, ct).ConfigureAwait(false); - // If RunAsync completes without throwing, the command succeeded. - // Checkpoint-Computer produces no output on success but throws on failure. - return true; + // Checkpoint-Computer does NOT always throw on failure — e.g. the once-per-24h + // rate limit writes a non-terminating error and produces no exception. Force the + // error to terminate and emit an explicit success sentinel only when it really + // succeeded, so a silent failure can no longer be reported as success. + var script = + "try { " + + $"Checkpoint-Computer -Description '{safeDesc}' -RestorePointType 'MODIFY_SETTINGS' -ErrorAction Stop; " + + "'__SM_RESTORE_OK__' " + + "} catch { Write-Error $_; exit 1 }"; + var results = await _ps.RunAsync(script, null, ct).ConfigureAwait(false); + var succeeded = results.Any(o => + string.Equals(o?.BaseObject?.ToString(), "__SM_RESTORE_OK__", StringComparison.Ordinal)); + if (!succeeded) + Log.Warning("CreateRestorePoint: Checkpoint-Computer did not confirm success (it may be rate-limited to one per 24h)."); + return succeeded; } // ═══════════════════════════════════════════════════════════════ diff --git a/SysManager/SysManager/Services/UpdateService.cs b/SysManager/SysManager/Services/UpdateService.cs index 41b29a5..bdf9b64 100644 --- a/SysManager/SysManager/Services/UpdateService.cs +++ b/SysManager/SysManager/Services/UpdateService.cs @@ -22,7 +22,17 @@ public sealed class UpdateService { public const string Owner = "laurentiu021"; public const string Repo = "SystemManager"; - public const string AssetName = "SysManager.exe"; + + /// + /// True for the release's main executable asset. Release assets are named + /// SysManager-v<version>.exe (e.g. SysManager-v1.20.1.exe), + /// not a fixed SysManager.exe, so match by pattern and exclude the + /// companion .sha256 checksum file. + /// + public static bool IsMainExeAsset(string? assetName) => + !string.IsNullOrEmpty(assetName) && + assetName.StartsWith("SysManager-", StringComparison.OrdinalIgnoreCase) && + assetName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase); private static readonly HttpClient Http = CreateClient(); @@ -345,8 +355,7 @@ private static void CleanupFile(string path) var version = ParseVersion(dto.TagName); if (version is null) return null; - var asset = dto.Assets?.FirstOrDefault(a => - string.Equals(a.Name, AssetName, StringComparison.OrdinalIgnoreCase)); + var asset = dto.Assets?.FirstOrDefault(a => IsMainExeAsset(a.Name)); return new ReleaseInfo( Version: version, diff --git a/SysManager/SysManager/Services/WindowsUpdateService.cs b/SysManager/SysManager/Services/WindowsUpdateService.cs index 7518399..eb029db 100644 --- a/SysManager/SysManager/Services/WindowsUpdateService.cs +++ b/SysManager/SysManager/Services/WindowsUpdateService.cs @@ -32,33 +32,43 @@ public Task> ScanAsync(CancellationToken ct = default { ct.ThrowIfCancellationRequested(); Emit("Connecting to Windows Update…"); - var session = CreateSession(); - var searcher = session.CreateUpdateSearcher(); - searcher.IncludePotentiallySupersededUpdates = false; - // "IsInstalled=0" returns everything not yet installed, - // including optional drivers and feature upgrades. - var result = searcher.Search("IsInstalled=0"); - ct.ThrowIfCancellationRequested(); + // Declared outside the try so the finally can release every COM object + // even when cancellation or MapToEntry throws mid-scan (previously these + // releases sat on the happy path only, leaking COM objects on any throw). + dynamic? session = null, searcher = null, result = null, updates = null; + try + { + session = CreateSession(); + searcher = session.CreateUpdateSearcher(); + searcher.IncludePotentiallySupersededUpdates = false; + + // "IsInstalled=0" returns everything not yet installed, + // including optional drivers and feature upgrades. + result = searcher.Search("IsInstalled=0"); + ct.ThrowIfCancellationRequested(); - var updates = result.Updates; - var count = (int)updates.Count; - Emit($"Found {count} update(s)."); + updates = result.Updates; + var count = (int)updates.Count; + Emit($"Found {count} update(s)."); - var list = new List(count); - for (int i = 0; i < count; i++) + var list = new List(count); + for (int i = 0; i < count; i++) + { + ct.ThrowIfCancellationRequested(); + var u = updates.Item(i); + list.Add(MapToEntry(u)); + Marshal.FinalReleaseComObject(u); + } + return list; + } + finally { - ct.ThrowIfCancellationRequested(); - var u = updates.Item(i); - list.Add(MapToEntry(u)); - Marshal.FinalReleaseComObject(u); + if (updates is not null) Marshal.FinalReleaseComObject(updates); + if (result is not null) Marshal.FinalReleaseComObject(result); + if (searcher is not null) Marshal.FinalReleaseComObject(searcher); + if (session is not null) Marshal.FinalReleaseComObject(session); } - - Marshal.FinalReleaseComObject(updates); - Marshal.FinalReleaseComObject(result); - Marshal.FinalReleaseComObject(searcher); - Marshal.FinalReleaseComObject(session); - return list; }, ct); /// diff --git a/SysManager/SysManager/SysManager.csproj b/SysManager/SysManager/SysManager.csproj index 26b297c..2cec3e0 100644 --- a/SysManager/SysManager/SysManager.csproj +++ b/SysManager/SysManager/SysManager.csproj @@ -10,9 +10,9 @@ SysManager true NU1603;NU1701 - 1.20.1 - 1.20.1.0 - 1.20.1.0 + 1.20.2 + 1.20.2.0 + 1.20.2.0 SysManager SysManager — Windows system monitoring toolkit by laurentiu021. Network, updates, health, logs, safe deep cleanup. https://github.com/laurentiu021/SystemManager