fix: code review batch 2 — IDialogService, QA/security fixes

CHANGELOG.md

@@ -4,6 +4,27 @@ All notable changes to this project are documented here. The format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project
 adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Added
+- **IDialogService** — abstraction for user confirmation dialogs, replacing
+  direct `MessageBox.Show` calls in ViewModels. Enables unit testing of
+  confirmation-gated code paths (CQ-003).
+
+### Fixed
+- **Disk Health** — `TemperatureColorHex` returns grey (#9AA0A6) for drives
+  without temperature sensors instead of misleading red (QA-004).
+- **Battery Health** — `HealthPercent` clamped to 0–100, `WearPercent`
+  clamped to ≥0 for new batteries exceeding design capacity (QA-005).
+- **Network Monitor** — `TrimBuffer` batch-removes expired points from
+  end-to-start, eliminating O(n²) array shifting (CQ-001).
+- **Shortcut Cleaner** — COM objects (`IShellLink`, `IPersistFile`) now
+  released via `Marshal.ReleaseComObject` in finally block (SEC-006).
+
+### Security
+- **Speed Test** — improved download integrity comment and added
+  Authenticode signature verification on extracted speedtest.exe (SEC-001).
+
 ## [0.35.11] - 2026-05-12
 
 ### Fixed

SysManager/SysManager.Tests/AppBlockerServiceValidationTests.cs

@@ -0,0 +1,70 @@
+// SysManager · AppBlockerService input validation tests (no registry access)
+using SysManager.Services;
+
+namespace SysManager.Tests;
+
+public class AppBlockerServiceValidationTests
+{
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    [InlineData("   ")]
+    public void BlockApp_NullOrWhitespace_ReturnsFalse(string? exeName)
+    {
+        Assert.False(AppBlockerService.BlockApp(exeName!));
+    }
+
+    [Theory]
+    [InlineData(@"..\..\windows\system32\cmd.exe")]
+    [InlineData(@"path\to\app.exe")]
+    [InlineData(@"folder/app.exe")]
+    [InlineData("app;malicious.exe")]
+    [InlineData("app&cmd.exe")]
+    [InlineData("app|pipe.exe")]
+    [InlineData("app<>.exe")]
+    [InlineData(@"app"".exe")]
+    public void BlockApp_InvalidCharsInName_ReturnsFalse(string exeName)
+    {
+        Assert.False(AppBlockerService.BlockApp(exeName));
+    }
+
+    [Theory]
+    [InlineData("notepad")]
+    [InlineData("notepad.exe")]
+    [InlineData("my-app.exe")]
+    [InlineData("My App.exe")]
+    [InlineData("app_v2.0.exe")]
+    public void BlockApp_ValidNames_FormatsCorrectly(string exeName)
+    {
+        // These are valid names but will fail because we don't have admin access.
+        // The important thing is they pass the validation check.
+        // The method returns false because of UnauthorizedAccessException,
+        // not because of invalid input. We can't distinguish in the return value,
+        // but we verify the format is accepted by checking IsBlocked (which also
+        // requires registry access and will return false gracefully).
+        var result = AppBlockerService.IsBlocked(exeName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase) ? exeName : exeName + ".exe");
+        // IsBlocked reads registry - will return false if no access, but won't throw
+        Assert.False(result); // expected since nothing is actually blocked
+    }
+
+    [Fact]
+    public void GetBlockedApps_ReturnsListWithoutThrowing()
+    {
+        // Should not throw even without admin
+        var result = AppBlockerService.GetBlockedApps();
+        Assert.NotNull(result);
+    }
+
+    [Fact]
+    public void IsBlocked_EmptyName_ReturnsFalse()
+    {
+        Assert.False(AppBlockerService.IsBlocked(""));
+    }
+
+    [Fact]
+    public void IsBlocked_NormalExeName_DoesNotThrow()
+    {
+        var ex = Record.Exception(() => AppBlockerService.IsBlocked("notepad.exe"));
+        Assert.Null(ex);
+    }
+}

SysManager/SysManager.Tests/BatteryInfoEdgeCaseTests.cs

@@ -0,0 +1,122 @@
+// SysManager · BatteryInfo computed property edge-case tests
+using SysManager.Models;
+
+namespace SysManager.Tests;
+
+public class BatteryInfoEdgeCaseTests
+{
+    [Fact]
+    public void HealthPercent_ZeroDesignCapacity_ReturnsZero()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 0, FullChargeCapacityMWh = 5000 };
+        Assert.Equal(0, info.HealthPercent);
+    }
+
+    [Fact]
+    public void HealthPercent_EqualCapacities_Returns100()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 50000, FullChargeCapacityMWh = 50000 };
+        Assert.Equal(100.0, info.HealthPercent);
+    }
+
+    [Fact]
+    public void HealthPercent_HalfWorn_Returns50()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 50000, FullChargeCapacityMWh = 25000 };
+        Assert.Equal(50.0, info.HealthPercent);
+    }
+
+    [Fact]
+    public void HealthPercent_NewBatteryOverDesign_ClampedTo100()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 40000, FullChargeCapacityMWh = 44000 };
+        // QA-005 fix: clamped to 100 max
+        Assert.Equal(100, info.HealthPercent);
+    }
+
+    [Fact]
+    public void WearPercent_ZeroDesignCapacity_ReturnsZero()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 0, FullChargeCapacityMWh = 5000 };
+        Assert.Equal(0, info.WearPercent);
+    }
+
+    [Fact]
+    public void WearPercent_EqualCapacities_ReturnsZero()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 50000, FullChargeCapacityMWh = 50000 };
+        Assert.Equal(0, info.WearPercent);
+    }
+
+    [Fact]
+    public void WearPercent_HalfWorn_Returns50()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 50000, FullChargeCapacityMWh = 25000 };
+        Assert.Equal(50.0, info.WearPercent);
+    }
+
+    [Fact]
+    public void WearPercent_NewBatteryOverDesign_ClampedToZero()
+    {
+        var info = new BatteryInfo { DesignCapacityMWh = 40000, FullChargeCapacityMWh = 44000 };
+        // QA-005 fix: clamped to 0 min
+        Assert.Equal(0, info.WearPercent);
+    }
+
+    [Fact]
+    public void RuntimeDisplay_PluggedIn_ShowsPluggedIn()
+    {
+        var info = new BatteryInfo { EstimatedRuntimeMinutes = -1 };
+        Assert.Equal("Plugged in", info.RuntimeDisplay);
+    }
+
+    [Fact]
+    public void RuntimeDisplay_Calculating_ShowsCalculating()
+    {
+        var info = new BatteryInfo { EstimatedRuntimeMinutes = 0 };
+        Assert.Contains("Calculating", info.RuntimeDisplay);
+    }
+
+    [Fact]
+    public void RuntimeDisplay_90Minutes_ShowsFormatted()
+    {
+        var info = new BatteryInfo { EstimatedRuntimeMinutes = 90 };
+        Assert.Equal("1h 30m", info.RuntimeDisplay);
+    }
+
+    [Fact]
+    public void RuntimeDisplay_60Minutes_ShowsFormatted()
+    {
+        var info = new BatteryInfo { EstimatedRuntimeMinutes = 60 };
+        Assert.Equal("1h 0m", info.RuntimeDisplay);
+    }
+
+    [Fact]
+    public void RuntimeDisplay_30Minutes_ShowsFormatted()
+    {
+        var info = new BatteryInfo { EstimatedRuntimeMinutes = 30 };
+        Assert.Equal("0h 30m", info.RuntimeDisplay);
+    }
+
+    [Fact]
+    public void PropertyChanged_FiredOnChargePercentChange()
+    {
+        var info = new BatteryInfo();
+        var changes = new List<string>();
+        info.PropertyChanged += (_, e) => changes.Add(e.PropertyName!);
+
+        info.ChargePercent = 75;
+
+        Assert.Contains("ChargePercent", changes);
+    }
+
+    [Fact]
+    public void DefaultValues_AllStringsEmpty()
+    {
+        var info = new BatteryInfo();
+        Assert.Equal("", info.Name);
+        Assert.Equal("", info.Status);
+        Assert.Equal("", info.Chemistry);
+        Assert.Equal("", info.Manufacturer);
+    }
+}

SysManager/SysManager.Tests/CleanupCategoryHumanSizeExtendedTests.cs

@@ -0,0 +1,156 @@
+// SysManager · CleanupCategory.HumanSize extended boundary tests
+using SysManager.Models;
+
+namespace SysManager.Tests;
+
+public class CleanupCategoryHumanSizeExtendedTests
+{
+    [Fact]
+    public void HumanSize_Zero_ReturnsZeroB()
+    {
+        Assert.Equal("0 B", CleanupCategory.HumanSize(0));
+    }
+
+    [Fact]
+    public void HumanSize_Negative_ReturnsZeroB()
+    {
+        Assert.Equal("0 B", CleanupCategory.HumanSize(-100));
+    }
+
+    [Fact]
+    public void HumanSize_OneByte_Returns1B()
+    {
+        Assert.Equal("1 B", CleanupCategory.HumanSize(1));
+    }
+
+    [Fact]
+    public void HumanSize_1023Bytes_Returns1023B()
+    {
+        Assert.Equal("1023 B", CleanupCategory.HumanSize(1023));
+    }
+
+    [Fact]
+    public void HumanSize_1024Bytes_Returns1KB()
+    {
+        Assert.Equal("1 KB", CleanupCategory.HumanSize(1024));
+    }
+
+    [Fact]
+    public void HumanSize_1MB_Returns1MB()
+    {
+        Assert.Equal("1 MB", CleanupCategory.HumanSize(1024 * 1024));
+    }
+
+    [Fact]
+    public void HumanSize_1GB_Returns1GB()
+    {
+        Assert.Equal("1 GB", CleanupCategory.HumanSize(1024L * 1024 * 1024));
+    }
+
+    [Fact]
+    public void HumanSize_1TB_Returns1TB()
+    {
+        Assert.Equal("1 TB", CleanupCategory.HumanSize(1024L * 1024 * 1024 * 1024));
+    }
+
+    [Fact]
+    public void HumanSize_1point5GB_FormatsCorrectly()
+    {
+        long bytes = (long)(1.5 * 1024 * 1024 * 1024);
+        Assert.Equal("1.5 GB", CleanupCategory.HumanSize(bytes));
+    }
+
+    [Fact]
+    public void HumanSize_LargeValue_StaysInTB()
+    {
+        long bytes = 10L * 1024 * 1024 * 1024 * 1024;
+        Assert.Equal("10 TB", CleanupCategory.HumanSize(bytes));
+    }
+
+    [Fact]
+    public void HumanSize_MaxLong_DoesNotThrow()
+    {
+        var result = CleanupCategory.HumanSize(long.MaxValue);
+        Assert.Contains("TB", result);
+    }
+
+    [Fact]
+    public void CleanupResult_Summary_NoErrors_FormatsCorrectly()
+    {
+        var result = new CleanupResult
+        {
+            BytesFreed = 1024 * 1024 * 50,
+            FilesDeleted = 100
+        };
+        Assert.Contains("50 MB", result.Summary);
+        Assert.Contains("100", result.Summary);
+        Assert.DoesNotContain("skipped", result.Summary);
+    }
+
+    [Fact]
+    public void CleanupResult_Summary_WithErrors_ShowsSkipped()
+    {
+        var result = new CleanupResult
+        {
+            BytesFreed = 1024 * 1024,
+            FilesDeleted = 10,
+            Errors = new[] { "err1", "err2" }
+        };
+        Assert.Contains("2 skipped", result.Summary);
+    }
+
+    [Fact]
+    public void LargeFileEntry_SizeDisplay_UsesHumanSize()
+    {
+        var entry = new LargeFileEntry
+        {
+            Path = @"C:\test\bigfile.iso",
+            Name = "bigfile.iso",
+            SizeBytes = 4_700_000_000,
+            LastModified = new DateTime(2024, 6, 15)
+        };
+        Assert.Contains("GB", entry.SizeDisplay);
+    }
+
+    [Fact]
+    public void LargeFileEntry_LastModifiedDisplay_FormatsCorrectly()
+    {
+        var entry = new LargeFileEntry
+        {
+            Path = @"C:\test\file.zip",
+            Name = "file.zip",
+            SizeBytes = 1000,
+            LastModified = new DateTime(2024, 3, 1)
+        };
+        Assert.Contains("2024", entry.LastModifiedDisplay);
+        Assert.Contains("Mar", entry.LastModifiedDisplay);
+    }
+
+    [Fact]
+    public void CleanupCategory_SizeDisplay_ShowsHumanSize()
+    {
+        var cat = new CleanupCategory
+        {
+            Name = "Test",
+            Description = "Test category",
+            Paths = new[] { @"C:\temp" },
+            TotalSizeBytes = 2048,
+            FileCount = 5
+        };
+        Assert.Equal("2 KB", cat.SizeDisplay);
+    }
+
+    [Fact]
+    public void CleanupCategory_CountDisplay_FormatsWithCommas()
+    {
+        var cat = new CleanupCategory
+        {
+            Name = "Test",
+            Description = "Test category",
+            Paths = new[] { @"C:\temp" },
+            TotalSizeBytes = 0,
+            FileCount = 1500
+        };
+        Assert.Contains("1,500", cat.CountDisplay);
+    }
+}