From 5ec0dd97e02e94c1231f1bfe0cbbb7d197de0928 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 1 Jul 2026 11:02:51 -0700 Subject: [PATCH 1/5] Validate --pluginassembly at parse time with a native FileInfo option Use Option with AcceptExistingOnly so a missing plugin path fails at argument parsing with a clear "File does not exist" message before the command runs, and simplify PluginLoader.Load to take a FileInfo and drop the empty-path, GetFullPath, and File.Exists guards (the CLI now guarantees an existing path). Scope is limited to --pluginassembly, the one path option that is a pure input whose validation this removes; --mediafiles/--settingsfile/--schemafile keep their current handling. Co-Authored-By: Claude Opus 4.8 (1M context) --- PlexCleaner/CommandLineOptions.cs | 12 ++++++++---- PlexCleaner/PluginLoader.cs | 21 ++++----------------- PlexCleaner/Program.cs | 7 ++++++- PlexCleanerTests/PluginLoaderTests.cs | 4 ++-- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/PlexCleaner/CommandLineOptions.cs b/PlexCleaner/CommandLineOptions.cs index c55ad54a..973d3d87 100644 --- a/PlexCleaner/CommandLineOptions.cs +++ b/PlexCleaner/CommandLineOptions.cs @@ -19,7 +19,7 @@ public class CommandLineOptions public string ResultsFile { get; set; } = string.Empty; public string SchemaFile { get; set; } = string.Empty; public string SettingsFile { get; set; } = string.Empty; - public string PluginAssembly { get; set; } = string.Empty; + public FileInfo? PluginAssembly { get; set; } } public class CommandLineParser @@ -131,7 +131,11 @@ private class CommandHandler(Func action) : SynchronousCommand HelpName = "boolean", }; - private readonly Option _pluginAssemblyOption = new("--pluginassembly") + // FileInfo with AcceptExistingOnly validates the path exists at parse time, so the plugin loader + // does not need to re-check it + private readonly Option _pluginAssemblyOption = new Option( + "--pluginassembly" + ) { Description = "Path to a plugin assembly implementing IProcessPlugin", HelpName = "filepath", @@ -139,7 +143,7 @@ private class CommandHandler(Func action) : SynchronousCommand #if PLUGINS Required = true, #endif - }; + }.AcceptExistingOnly(); private RootCommand CreateRootCommand() { @@ -440,7 +444,7 @@ public CommandLineOptions Bind() ResultsFile = Result.GetValue(_resultsFileOption) ?? string.Empty, SchemaFile = Result.GetValue(_schemaFileOption) ?? string.Empty, SettingsFile = Result.GetValue(_settingsFileOption) ?? string.Empty, - PluginAssembly = Result.GetValue(_pluginAssemblyOption) ?? string.Empty, + PluginAssembly = Result.GetValue(_pluginAssemblyOption), }; return options; diff --git a/PlexCleaner/PluginLoader.cs b/PlexCleaner/PluginLoader.cs index 6cc3f66f..870b4a82 100644 --- a/PlexCleaner/PluginLoader.cs +++ b/PlexCleaner/PluginLoader.cs @@ -59,26 +59,13 @@ public static class PluginLoader "Loads a plugin assembly and discovers IProcessPlugin via reflection" )] [RequiresDynamicCode("Loads a plugin assembly at runtime")] - public static IProcessPlugin? Load(string assemblyPath) + public static IProcessPlugin? Load(FileInfo assemblyFile) { - // An empty path would resolve to the current directory and report a misleading error - if (string.IsNullOrWhiteSpace(assemblyPath)) - { - Log.Error("Plugin assembly path is empty"); - return null; - } - - // Resolve inside the try so a malformed path (GetFullPath throws) fails cleanly with a log - string fullPath = assemblyPath; + // The CLI validates that the path exists (AcceptExistingOnly), so use the resolved full path + // directly and let the try/catch handle assembly load and reflection errors + string fullPath = assemblyFile.FullName; try { - fullPath = Path.GetFullPath(assemblyPath); - if (!File.Exists(fullPath)) - { - Log.Error("Plugin assembly not found : {AssemblyPath}", fullPath); - return null; - } - PluginLoadContext context = new(fullPath); Assembly assembly = context.LoadFromAssemblyPath(fullPath); diff --git a/PlexCleaner/Program.cs b/PlexCleaner/Program.cs index e745f40b..e7c39082 100644 --- a/PlexCleaner/Program.cs +++ b/PlexCleaner/Program.cs @@ -285,7 +285,12 @@ public static int CustomCommand() return MakeExitCode(ExitCode.Error); } - // Load the plugin after config and tools are initialized + // Load the plugin after config and tools are initialized (the option is required, so the + // path is always present here) + if (Options.PluginAssembly == null) + { + return MakeExitCode(ExitCode.Error); + } IProcessPlugin? plugin = PluginLoader.Load(Options.PluginAssembly); if (plugin == null) { diff --git a/PlexCleanerTests/PluginLoaderTests.cs b/PlexCleanerTests/PluginLoaderTests.cs index 5a5d17ef..6b1fd3a1 100644 --- a/PlexCleanerTests/PluginLoaderTests.cs +++ b/PlexCleanerTests/PluginLoaderTests.cs @@ -12,7 +12,7 @@ public class PluginLoaderTests [Fact] public void Load_ExamplePlugin_ReturnsInitializedPlugin() { - IProcessPlugin? plugin = PluginLoader.Load(ExamplePluginPath); + IProcessPlugin? plugin = PluginLoader.Load(new FileInfo(ExamplePluginPath)); // A non-null typed return proves the loaded type satisfies the host's IProcessPlugin // (single shared type identity) and that Initialize accepted the host @@ -26,7 +26,7 @@ public void Load_ExamplePlugin_ReturnsInitializedPlugin() public void Load_MissingAssembly_ReturnsNull() { IProcessPlugin? plugin = PluginLoader.Load( - Path.Combine(AppContext.BaseDirectory, "DoesNotExist.dll") + new FileInfo(Path.Combine(AppContext.BaseDirectory, "DoesNotExist.dll")) ); _ = plugin.Should().BeNull(); From e0940613b228d7c3c4e5a16287d94313b0025d38 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 1 Jul 2026 11:06:47 -0700 Subject: [PATCH 2/5] Resolve plugin FullName inside the try block FileInfo.FullName resolves the path and can throw on a malformed or too-long path; move it inside the try so any error is logged via LogAndHandle and returns null instead of throwing, and log the file name in the catch since it does not resolve the path. Co-Authored-By: Claude Opus 4.8 (1M context) --- PlexCleaner/PluginLoader.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/PlexCleaner/PluginLoader.cs b/PlexCleaner/PluginLoader.cs index 870b4a82..47e2ae95 100644 --- a/PlexCleaner/PluginLoader.cs +++ b/PlexCleaner/PluginLoader.cs @@ -61,11 +61,11 @@ public static class PluginLoader [RequiresDynamicCode("Loads a plugin assembly at runtime")] public static IProcessPlugin? Load(FileInfo assemblyFile) { - // The CLI validates that the path exists (AcceptExistingOnly), so use the resolved full path - // directly and let the try/catch handle assembly load and reflection errors - string fullPath = assemblyFile.FullName; + // The CLI validates that the path exists (AcceptExistingOnly), so just load and reflect. Resolve + // FullName inside the try so any path error is logged and returns null instead of throwing. try { + string fullPath = assemblyFile.FullName; PluginLoadContext context = new(fullPath); Assembly assembly = context.LoadFromAssemblyPath(fullPath); @@ -107,8 +107,8 @@ .. assembly } catch (Exception e) when (Log.Logger.LogAndHandle(e)) { - // Include the assembly path since LogAndHandle only reports the caller member - Log.Error("Failed to load plugin : {AssemblyPath}", fullPath); + // Log the file name (does not resolve the path) since LogAndHandle only reports the caller + Log.Error("Failed to load plugin : {AssemblyName}", assemblyFile.Name); return null; } } From 990173c4db7aa94342c6c831644d712ab78cca54 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 1 Jul 2026 11:13:00 -0700 Subject: [PATCH 3/5] Harden plugin null handling and add custom command parse tests - PluginLoader.Load: guard against a null FileInfo with ArgumentNullException so the catch block never dereferences a null argument - CustomCommand: log an explicit error before returning when the plugin path is missing so the failure is visible even though the option is required - CommandLineTests: cover the custom command, an existing plugin path binds and a missing path fails at parse time via AcceptExistingOnly Co-Authored-By: Claude Opus 4.8 (1M context) --- PlexCleaner/PluginLoader.cs | 2 ++ PlexCleaner/Program.cs | 1 + PlexCleanerTests/CommandLineTests.cs | 39 ++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/PlexCleaner/PluginLoader.cs b/PlexCleaner/PluginLoader.cs index 47e2ae95..25fdd9aa 100644 --- a/PlexCleaner/PluginLoader.cs +++ b/PlexCleaner/PluginLoader.cs @@ -61,6 +61,8 @@ public static class PluginLoader [RequiresDynamicCode("Loads a plugin assembly at runtime")] public static IProcessPlugin? Load(FileInfo assemblyFile) { + ArgumentNullException.ThrowIfNull(assemblyFile); + // The CLI validates that the path exists (AcceptExistingOnly), so just load and reflect. Resolve // FullName inside the try so any path error is logged and returns null instead of throwing. try diff --git a/PlexCleaner/Program.cs b/PlexCleaner/Program.cs index e7c39082..e7fcef36 100644 --- a/PlexCleaner/Program.cs +++ b/PlexCleaner/Program.cs @@ -289,6 +289,7 @@ public static int CustomCommand() // path is always present here) if (Options.PluginAssembly == null) { + Log.Error("Plugin assembly path is required"); return MakeExitCode(ExitCode.Error); } IProcessPlugin? plugin = PluginLoader.Load(Options.PluginAssembly); diff --git a/PlexCleanerTests/CommandLineTests.cs b/PlexCleanerTests/CommandLineTests.cs index ea13e91b..108ca85d 100644 --- a/PlexCleanerTests/CommandLineTests.cs +++ b/PlexCleanerTests/CommandLineTests.cs @@ -383,6 +383,45 @@ public void Parse_Commandline_TestMediaInfo(params string[] args) _ = options.ThreadCount.Should().Be(2); } + [Fact] + public void Parse_Commandline_Custom_ExistingPluginAssembly_Binds() + { + // AcceptExistingOnly validates the path at parse time, so use a file that exists + string existing = typeof(CommandLineTests).Assembly.Location; + string[] args = + [ + "custom", + "--settingsfile=settings.json", + $"--pluginassembly={existing}", + "--mediafiles=/data/foo", + ]; + + CommandLineParser parser = new(args); + _ = parser.Result.Errors.Should().BeEmpty(); + _ = parser.Result.CommandResult.Command.Name.Should().Be("custom"); + + CommandLineOptions options = parser.Bind(); + FileInfo? pluginAssembly = options.PluginAssembly; + _ = pluginAssembly.Should().NotBeNull(); + _ = pluginAssembly.FullName.Should().Be(new FileInfo(existing).FullName); + } + + [Fact] + public void Parse_Commandline_Custom_MissingPluginAssembly_Fails() + { + // A non-existent plugin path fails at parse time via AcceptExistingOnly + string[] args = + [ + "custom", + "--settingsfile=settings.json", + "--pluginassembly=/does/not/exist.dll", + "--mediafiles=/data/foo", + ]; + + CommandLineParser parser = new(args); + _ = parser.Result.Errors.Should().NotBeEmpty(); + } + [Theory] [InlineData("--help")] [InlineData("--version")] From f8a7a6a63943965918285ebf0569ae8a33152d3c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 1 Jul 2026 11:18:30 -0700 Subject: [PATCH 4/5] Log the resolved plugin path in the load failure message Track the assembly path in a single variable that starts as the file name and upgrades to the resolved full path inside the try, so the catch logs the most specific location available (full path when resolved, file name if FullName threw) instead of only the ambiguous file name. Co-Authored-By: Claude Opus 4.8 (1M context) --- PlexCleaner/PluginLoader.cs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/PlexCleaner/PluginLoader.cs b/PlexCleaner/PluginLoader.cs index 25fdd9aa..2544ee00 100644 --- a/PlexCleaner/PluginLoader.cs +++ b/PlexCleaner/PluginLoader.cs @@ -63,13 +63,14 @@ public static class PluginLoader { ArgumentNullException.ThrowIfNull(assemblyFile); - // The CLI validates that the path exists (AcceptExistingOnly), so just load and reflect. Resolve - // FullName inside the try so any path error is logged and returns null instead of throwing. + // Start with the safe file name and upgrade to the resolved full path inside the try, so the + // catch can log the most specific location available without FullName throwing before the try + string assemblyPath = assemblyFile.Name; try { - string fullPath = assemblyFile.FullName; - PluginLoadContext context = new(fullPath); - Assembly assembly = context.LoadFromAssemblyPath(fullPath); + assemblyPath = assemblyFile.FullName; + PluginLoadContext context = new(assemblyPath); + Assembly assembly = context.LoadFromAssemblyPath(assemblyPath); // Require exactly one concrete IProcessPlugin implementation List pluginTypes = @@ -86,7 +87,7 @@ .. assembly Log.Error( "Plugin assembly must contain exactly one IProcessPlugin implementation, found {Count} : {AssemblyPath}", pluginTypes.Count, - fullPath + assemblyPath ); return null; } @@ -104,13 +105,14 @@ .. assembly return null; } - Log.Information("Loaded plugin : {Name} : {AssemblyPath}", plugin.Name, fullPath); + Log.Information("Loaded plugin : {Name} : {AssemblyPath}", plugin.Name, assemblyPath); return plugin; } catch (Exception e) when (Log.Logger.LogAndHandle(e)) { - // Log the file name (does not resolve the path) since LogAndHandle only reports the caller - Log.Error("Failed to load plugin : {AssemblyName}", assemblyFile.Name); + // Log the resolved path (falls back to the file name if FullName threw) since LogAndHandle + // only reports the caller member + Log.Error("Failed to load plugin : {AssemblyPath}", assemblyPath); return null; } } From 54f0cc51ac4beaac797ef19fb0241d22be1d8eec Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 1 Jul 2026 11:23:32 -0700 Subject: [PATCH 5/5] Gate --pluginassembly existence check to non-AOT builds AcceptExistingOnly was applied unconditionally, so under an AOT build a non-existent plugin path would fail at parse instead of reaching CustomCommand and its "requires a non-AOT build" message. Gate both Required and AcceptExistingOnly behind PLUGINS, matching the command handler. Co-Authored-By: Claude Opus 4.8 (1M context) --- PlexCleaner/CommandLineOptions.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/PlexCleaner/CommandLineOptions.cs b/PlexCleaner/CommandLineOptions.cs index 973d3d87..5dc9335c 100644 --- a/PlexCleaner/CommandLineOptions.cs +++ b/PlexCleaner/CommandLineOptions.cs @@ -132,18 +132,24 @@ private class CommandHandler(Func action) : SynchronousCommand }; // FileInfo with AcceptExistingOnly validates the path exists at parse time, so the plugin loader - // does not need to re-check it + // does not need to re-check it. Both Required and the existence check are gated to non-AOT builds so + // that under AOT the custom command still reaches CustomCommand and emits the non-AOT error. +#if PLUGINS private readonly Option _pluginAssemblyOption = new Option( "--pluginassembly" ) { Description = "Path to a plugin assembly implementing IProcessPlugin", HelpName = "filepath", - // Not required in AOT builds so custom can reach CustomCommand and emit the non-AOT error -#if PLUGINS Required = true, -#endif }.AcceptExistingOnly(); +#else + private readonly Option _pluginAssemblyOption = new("--pluginassembly") + { + Description = "Path to a plugin assembly implementing IProcessPlugin", + HelpName = "filepath", + }; +#endif private RootCommand CreateRootCommand() {