diff --git a/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs b/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs index 655d4d9e..e9736463 100644 --- a/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs +++ b/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs @@ -1,5 +1,7 @@ using System.Reflection; +using Newtonsoft.Json; + using NUnit.Framework; namespace LogExpert.PluginRegistry.Tests; @@ -16,6 +18,9 @@ namespace LogExpert.PluginRegistry.Tests; /// the type is already loaded by the time any in-process test runs, and an eager load /// targets the real %APPDATA% path. Keep PluginValidator free of a static constructor and /// of I/O in field initializers. +/// +/// Also covers issue #733: loading the configuration refreshes built-in plugin hashes +/// persisted by an older build, keeping user-trusted entries. /// [TestFixture] public class PluginValidatorConfigPersistenceTests @@ -26,6 +31,9 @@ public class PluginValidatorConfigPersistenceTests private string _originalConfigDirectory = null!; private string _originalConfigPath = null!; private object? _originalConfig; + private bool _originalBypassSetting; + + private const string STALE_HASH = "0000000000000000000000000000000000000000000000000000000000000000"; private static FieldInfo GetStaticField (string name) { @@ -44,6 +52,7 @@ public void SetUp () _originalConfigDirectory = (string)GetStaticField("_configDirectory").GetValue(null)!; _originalConfigPath = (string)GetStaticField("_configPath").GetValue(null)!; _originalConfig = GetStaticField("_trustedPluginConfig").GetValue(null); + _originalBypassSetting = PluginHashCalculator.BypassHashVerification; } [TearDown] @@ -53,6 +62,7 @@ public void TearDown () GetStaticField("_configDirectory").SetValue(null, _originalConfigDirectory); GetStaticField("_configPath").SetValue(null, _originalConfigPath); GetStaticField("_trustedPluginConfig").SetValue(null, _originalConfig); + PluginHashCalculator.BypassHashVerification = _originalBypassSetting; if (Directory.Exists(_testDataPath)) { @@ -78,6 +88,40 @@ private void SimulatePreInitializeState () GetStaticField("_trustedPluginConfig").SetValue(null, null); } + private static string BuiltCsvColumnizerPath () + { + // Test output is src/PluginRegistry.Tests/bin/; plugins are built to bin//plugins. + var binDirectory = Path.GetDirectoryName(typeof(PluginValidatorConfigPersistenceTests).Assembly.Location)!; + var path = Path.GetFullPath(Path.Join(binDirectory, "..", "..", "..", "..", "bin", Path.GetFileName(binDirectory), "plugins", "CsvColumnizer.dll")); + Assert.That(File.Exists(path), Is.True, $"Built plugin not found: {path}"); + return path; + } + + private void SeedConfig (TrustedPluginConfig config) + { + File.WriteAllText(Path.Join(_configDir, "trusted-plugins.json"), JsonConvert.SerializeObject(config)); + } + + private TrustedPluginConfig ReadPersistedConfig () + { + return JsonConvert.DeserializeObject(File.ReadAllText(Path.Join(_configDir, "trusted-plugins.json")))!; + } + + private static TrustedPluginConfig StaleConfig () + { + return new TrustedPluginConfig + { + PluginNames = ["CsvColumnizer.dll", "UserPlugin.dll"], + PluginHashes = new Dictionary + { + ["CsvColumnizer.dll"] = STALE_HASH, + ["CsvColumnizer.dll (x86)"] = STALE_HASH, + ["UserPlugin.dll"] = "ABCDEF" + }, + AllowUserTrustedPlugins = false + }; + } + [Test] [Description("Issue #658: config must be loaded lazily on first use, not eagerly at class load")] public void ValidatePlugin_WhenConfigNotYetLoaded_LoadsLazilyFromConfiguredDirectory () @@ -132,4 +176,61 @@ public void Initialize_WithEmptyDirectory_CreatesDefaultConfigFileThere () "Nothing may be written to the pre-Initialize default directory"); }); } + + [Test] + [Description("Issue #733: a stale persisted hash for a built-in plugin must not reject the plugin after an upgrade")] + public void Initialize_WithStaleBuiltInHash_ValidatesBuiltInPlugin () + { + PluginHashCalculator.BypassHashVerification = false; + SeedConfig(StaleConfig()); + + PluginValidator.Initialize(_configDir); + + var result = PluginValidator.ValidatePlugin(BuiltCsvColumnizerPath(), out _, out var errorMessage); + Assert.That(result, Is.True, $"Built-in plugin should validate, but failed with: {errorMessage}"); + } + + [Test] + [Description("Issue #733: the refreshed built-in hashes are persisted; user entries and settings survive")] + public void Initialize_WithStaleBuiltInHash_PersistsRunningBuildHashesAndKeepsUserEntries () + { + SeedConfig(StaleConfig()); + + PluginValidator.Initialize(_configDir); + + var persisted = ReadPersistedConfig(); + var builtIn = PluginValidator.GetBuiltInPluginHashes(); + Assert.Multiple(() => + { + Assert.That(persisted.PluginHashes["CsvColumnizer.dll"], Is.EqualTo(builtIn["CsvColumnizer.dll"])); + Assert.That(persisted.PluginHashes["CsvColumnizer.dll (x86)"], Is.EqualTo(builtIn["CsvColumnizer.dll (x86)"])); + Assert.That(persisted.PluginNames, Does.Contain("JsonColumnizer.dll"), "Missing built-in names must be added"); + Assert.That(persisted.PluginNames, Does.Contain("UserPlugin.dll")); + Assert.That(persisted.PluginHashes["UserPlugin.dll"], Is.EqualTo("ABCDEF")); + Assert.That(persisted.AllowUserTrustedPlugins, Is.False); + }); + } + + [Test] + [Description("Issue #733: a built-in plugin whose bytes differ from the running build's table is still rejected")] + public void ValidatePlugin_TamperedBuiltInPlugin_IsRejectedWithHashMismatch () + { + PluginHashCalculator.BypassHashVerification = false; + SeedConfig(StaleConfig()); + PluginValidator.Initialize(_configDir); + + var tamperedPath = Path.Join(_testDataPath, "CsvColumnizer.dll"); + File.Copy(BuiltCsvColumnizerPath(), tamperedPath); + using (var stream = File.Open(tamperedPath, FileMode.Append)) + { + stream.WriteByte(0); + } + + var result = PluginValidator.ValidatePlugin(tamperedPath, out _, out var errorMessage); + Assert.Multiple(() => + { + Assert.That(result, Is.False); + Assert.That(errorMessage, Does.Contain("has been modified")); + }); + } } diff --git a/src/PluginRegistry/PluginValidator.cs b/src/PluginRegistry/PluginValidator.cs index 00cdd9f9..9b85922e 100644 --- a/src/PluginRegistry/PluginValidator.cs +++ b/src/PluginRegistry/PluginValidator.cs @@ -120,6 +120,11 @@ private static void LoadTrustedPluginConfiguration () _trustedPluginConfig = CreateDefaultConfiguration(); SaveTrustedPluginConfiguration(); } + else if (RefreshBuiltInPlugins(_trustedPluginConfig)) + { + _logger.Info("Refreshed built-in plugin hashes from the running build"); + SaveTrustedPluginConfiguration(); + } } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or @@ -160,6 +165,35 @@ private static TrustedPluginConfig CreateDefaultConfiguration () }; } + /// + /// Makes the running build's built-in names and hashes authoritative in a loaded configuration, + /// so an upgrade does not reject the new built-in plugins against hashes persisted by an older build. + /// + /// True if the configuration was changed + private static bool RefreshBuiltInPlugins (TrustedPluginConfig config) + { + var changed = false; + + foreach (var name in _trustedPluginNames.Where(n => !config.PluginNames.Contains(n, StringComparer.OrdinalIgnoreCase))) + { + config.PluginNames.Add(name); + changed = true; + } + + foreach (var (name, hash) in GetBuiltInPluginHashes().Where(e => !config.PluginHashes.TryGetValue(e.Key, out var persistedHash) || !string.Equals(persistedHash, e.Value, StringComparison.OrdinalIgnoreCase))) + { + config.PluginHashes[name] = hash; + changed = true; + } + + if (changed) + { + config.LastUpdated = DateTime.UtcNow; + } + + return changed; + } + /// /// Saves trusted plugin configuration to disk. /// diff --git a/src/docs/PLUGIN_HASH_MANAGEMENT.md b/src/docs/PLUGIN_HASH_MANAGEMENT.md index b60c73e8..b8147765 100644 --- a/src/docs/PLUGIN_HASH_MANAGEMENT.md +++ b/src/docs/PLUGIN_HASH_MANAGEMENT.md @@ -59,9 +59,10 @@ The generated file does not exist yet on a fresh clone. Build once and it appear ### "Plugin hash mismatch" on a user's machine after an update -`trusted-plugins.json` in the user profile is seeded from `GetBuiltInPluginHashes()` on first launch -and is not rewritten by an upgrade, so hashes from the previous version linger. Re-trust the plugin -via Settings > Plugin Management, or delete `%APPDATA%\LogExpert\trusted-plugins.json` when testing. +Every load of `trusted-plugins.json` reconciles the built-in names and hashes with the running build's +`GetBuiltInPluginHashes()` (user-trusted entries are kept), so an upgrade does not cause this for +built-in plugins. A mismatch on a built-in plugin therefore means its bytes differ from the build +that is running: a partial or mixed install, or a modified file. Reinstall. ## Adding a plugin