From a2776faeb3d71002a37bf3942dfbab0e4c2bc0fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=AA=20Trung=20Hi=E1=BA=BFu?= Date: Wed, 8 Jul 2026 09:15:49 +0700 Subject: [PATCH] feat: harden updater error handling and input validation - Validate plugin/new-plugin/updater paths and extensions in PluginUpdater.TryScheduleUpdate. - Validate KeePass process ID is non-negative. - Add distinct exit codes and argument validation in KeePassAutoReload.Updater Program.cs. - Add directory creation before copying and verify restart executable exists. - Add 5 unit tests for invalid PluginUpdater inputs; dotnet test passes 71/71. Closes #46. --- src/KeePassAutoReload.Updater/Program.cs | 49 +++++++++++++++++++++--- src/PluginUpdater.cs | 10 +++++ tests/Tests.cs | 40 +++++++++++++++++++ 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/KeePassAutoReload.Updater/Program.cs b/src/KeePassAutoReload.Updater/Program.cs index 8a0e677..34eab88 100644 --- a/src/KeePassAutoReload.Updater/Program.cs +++ b/src/KeePassAutoReload.Updater/Program.cs @@ -7,6 +7,11 @@ namespace KeePassAutoReload.Updater { internal static class Program { + internal const int ExitSuccess = 0; + internal const int ExitInvalidArguments = 1; + internal const int ExitUpdateFailed = 2; + internal const int ExitRestartFailed = 3; + internal static int Main(string[] args) { int processId = 0; @@ -22,7 +27,11 @@ internal static int Main(string[] args) if (string.Equals(current, "--process-id", StringComparison.OrdinalIgnoreCase)) { - int.TryParse(value, out processId); + if (!int.TryParse(value, out processId) || processId < 0) + { + Console.Error.WriteLine("Invalid process ID."); + return ExitInvalidArguments; + } } else if (string.Equals(current, "--source", StringComparison.OrdinalIgnoreCase)) { @@ -41,7 +50,25 @@ internal static int Main(string[] args) if (string.IsNullOrWhiteSpace(source) || string.IsNullOrWhiteSpace(destination)) { Console.Error.WriteLine("Usage: KeePassAutoReload.Updater --source --destination [--process-id ] [--restart ]"); - return 1; + return ExitInvalidArguments; + } + + if (!source.EndsWith(".new", StringComparison.OrdinalIgnoreCase)) + { + Console.Error.WriteLine("Source file must have a .new extension."); + return ExitInvalidArguments; + } + + if (!destination.EndsWith(".dll", StringComparison.OrdinalIgnoreCase)) + { + Console.Error.WriteLine("Destination file must have a .dll extension."); + return ExitInvalidArguments; + } + + if (!File.Exists(source)) + { + Console.Error.WriteLine("Source file does not exist: " + source); + return ExitInvalidArguments; } try @@ -63,20 +90,30 @@ internal static int Main(string[] args) Thread.Sleep(1000); + string destinationDirectory = Path.GetDirectoryName(destination); + if (!string.IsNullOrWhiteSpace(destinationDirectory) && !Directory.Exists(destinationDirectory)) + { + Directory.CreateDirectory(destinationDirectory); + } + File.Copy(source, destination, overwrite: true); File.Delete(source); - if (!string.IsNullOrWhiteSpace(restart) && File.Exists(restart)) + if (string.IsNullOrWhiteSpace(restart)) return ExitSuccess; + + if (!File.Exists(restart)) { - Process.Start(restart); + Console.Error.WriteLine("KeePass executable not found: " + restart); + return ExitRestartFailed; } - return 0; + Process.Start(restart); + return ExitSuccess; } catch (Exception ex) { Console.Error.WriteLine("Update failed: " + ex.Message); - return 2; + return ExitUpdateFailed; } } } diff --git a/src/PluginUpdater.cs b/src/PluginUpdater.cs index 5d2a43b..c3039a6 100644 --- a/src/PluginUpdater.cs +++ b/src/PluginUpdater.cs @@ -35,6 +35,16 @@ public static bool TryScheduleUpdate( if (string.IsNullOrWhiteSpace(newPluginPath)) throw new ArgumentException("newPluginPath"); if (string.IsNullOrWhiteSpace(updaterExePath)) throw new ArgumentException("updaterExePath"); if (starter == null) throw new ArgumentNullException("starter"); + if (keepassProcessId < 0) throw new ArgumentOutOfRangeException("keepassProcessId"); + + if (!pluginPath.EndsWith(".dll", StringComparison.OrdinalIgnoreCase)) + throw new ArgumentException("pluginPath must end with .dll", "pluginPath"); + if (!newPluginPath.EndsWith(".new", StringComparison.OrdinalIgnoreCase)) + throw new ArgumentException("newPluginPath must end with .new", "newPluginPath"); + if (!updaterExePath.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)) + throw new ArgumentException("updaterExePath must end with .exe", "updaterExePath"); + if (!string.IsNullOrWhiteSpace(keepassExecutablePath) && !keepassExecutablePath.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)) + throw new ArgumentException("keepassExecutablePath must end with .exe", "keepassExecutablePath"); if (!File.Exists(newPluginPath)) return false; if (!File.Exists(updaterExePath)) return false; diff --git a/tests/Tests.cs b/tests/Tests.cs index e004525..da5d20a 100644 --- a/tests/Tests.cs +++ b/tests/Tests.cs @@ -524,6 +524,46 @@ public void TryScheduleUpdate_ThrowsWhenUpdaterPathIsInvalid(string updaterPath) PluginUpdater.TryScheduleUpdate(@"C:\plugin.dll", @"C:\new.dll", updaterPath, 1234, @"C:\KeePass.exe", starter)); } + [Fact] + public void TryScheduleUpdate_ThrowsWhenPluginPathDoesNotEndWithDll() + { + FakeProcessStarter starter = new FakeProcessStarter(); + Assert.Throws(() => + PluginUpdater.TryScheduleUpdate(@"C:\plugin.txt", @"C:\new.dll.new", @"C:\updater.exe", 1234, @"C:\KeePass.exe", starter)); + } + + [Fact] + public void TryScheduleUpdate_ThrowsWhenNewPluginPathDoesNotEndWithNew() + { + FakeProcessStarter starter = new FakeProcessStarter(); + Assert.Throws(() => + PluginUpdater.TryScheduleUpdate(@"C:\plugin.dll", @"C:\new.dll", @"C:\updater.exe", 1234, @"C:\KeePass.exe", starter)); + } + + [Fact] + public void TryScheduleUpdate_ThrowsWhenUpdaterPathDoesNotEndWithExe() + { + FakeProcessStarter starter = new FakeProcessStarter(); + Assert.Throws(() => + PluginUpdater.TryScheduleUpdate(@"C:\plugin.dll", @"C:\new.dll.new", @"C:\updater.bat", 1234, @"C:\KeePass.exe", starter)); + } + + [Fact] + public void TryScheduleUpdate_ThrowsWhenKeePassExecutablePathHasInvalidExtension() + { + FakeProcessStarter starter = new FakeProcessStarter(); + Assert.Throws(() => + PluginUpdater.TryScheduleUpdate(@"C:\plugin.dll", @"C:\new.dll.new", @"C:\updater.exe", 1234, @"C:\KeePass.txt", starter)); + } + + [Fact] + public void TryScheduleUpdate_ThrowsWhenProcessIdIsNegative() + { + FakeProcessStarter starter = new FakeProcessStarter(); + Assert.Throws(() => + PluginUpdater.TryScheduleUpdate(@"C:\plugin.dll", @"C:\new.dll.new", @"C:\updater.exe", -1, @"C:\KeePass.exe", starter)); + } + [Fact] public void TryScheduleUpdate_ThrowsWhenStarterIsNull() {