From c62eb9a73277d373f1137ad10d2ab6e341103a03 Mon Sep 17 00:00:00 2001 From: BRUNER Patrick Date: Mon, 19 Oct 2026 13:36:03 +0200 Subject: [PATCH 1/3] Refresh built-in plugin hashes when loading trusted-plugins.json (#733) trusted-plugins.json snapshotted the built-in hash table on first run and was never refreshed, so after an upgrade every built-in plugin failed hash verification. On load, the running build's built-in names and hashes now overwrite stale entries; user-trusted entries and settings are kept, and the file is saved back when anything changed. Tampered built-in DLLs are still rejected. --- .../PluginValidatorConfigPersistenceTests.cs | 98 +++++++++++++++++++ src/PluginRegistry/PluginValidator.cs | 40 ++++++++ src/docs/PLUGIN_HASH_MANAGEMENT.md | 7 +- 3 files changed, 142 insertions(+), 3 deletions(-) diff --git a/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs b/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs index 655d4d9e..10a96ff1 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; @@ -26,6 +28,9 @@ public class PluginValidatorConfigPersistenceTests private string _originalConfigDirectory = null!; private string _originalConfigPath = null!; private object? _originalConfig; + private bool _originalBypass; + + private const string STALE_HASH = "0000000000000000000000000000000000000000000000000000000000000000"; private static FieldInfo GetStaticField (string name) { @@ -44,6 +49,7 @@ public void SetUp () _originalConfigDirectory = (string)GetStaticField("_configDirectory").GetValue(null)!; _originalConfigPath = (string)GetStaticField("_configPath").GetValue(null)!; _originalConfig = GetStaticField("_trustedPluginConfig").GetValue(null); + _originalBypass = PluginHashCalculator.BypassHashVerification; } [TearDown] @@ -53,6 +59,7 @@ public void TearDown () GetStaticField("_configDirectory").SetValue(null, _originalConfigDirectory); GetStaticField("_configPath").SetValue(null, _originalConfigPath); GetStaticField("_trustedPluginConfig").SetValue(null, _originalConfig); + PluginHashCalculator.BypassHashVerification = _originalBypass; if (Directory.Exists(_testDataPath)) { @@ -78,6 +85,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 +173,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..c2b140c7 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,41 @@ 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) + { + if (!config.PluginNames.Contains(name, StringComparer.OrdinalIgnoreCase)) + { + config.PluginNames.Add(name); + changed = true; + } + } + + foreach (var (name, hash) in GetBuiltInPluginHashes()) + { + if (!config.PluginHashes.TryGetValue(name, out var persistedHash) || persistedHash != hash) + { + 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 From b73d79b90f045f1a0a32b094381f0d13b099b162 Mon Sep 17 00:00:00 2001 From: BRUNER Patrick Date: Mon, 19 Oct 2026 14:50:50 +0200 Subject: [PATCH 2/3] Address review: case-insensitive hash compare, align test naming, document #733 coverage --- .../PluginValidatorConfigPersistenceTests.cs | 9 ++++++--- src/PluginRegistry/PluginValidator.cs | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs b/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs index 10a96ff1..e9736463 100644 --- a/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs +++ b/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs @@ -18,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 @@ -28,7 +31,7 @@ public class PluginValidatorConfigPersistenceTests private string _originalConfigDirectory = null!; private string _originalConfigPath = null!; private object? _originalConfig; - private bool _originalBypass; + private bool _originalBypassSetting; private const string STALE_HASH = "0000000000000000000000000000000000000000000000000000000000000000"; @@ -49,7 +52,7 @@ public void SetUp () _originalConfigDirectory = (string)GetStaticField("_configDirectory").GetValue(null)!; _originalConfigPath = (string)GetStaticField("_configPath").GetValue(null)!; _originalConfig = GetStaticField("_trustedPluginConfig").GetValue(null); - _originalBypass = PluginHashCalculator.BypassHashVerification; + _originalBypassSetting = PluginHashCalculator.BypassHashVerification; } [TearDown] @@ -59,7 +62,7 @@ public void TearDown () GetStaticField("_configDirectory").SetValue(null, _originalConfigDirectory); GetStaticField("_configPath").SetValue(null, _originalConfigPath); GetStaticField("_trustedPluginConfig").SetValue(null, _originalConfig); - PluginHashCalculator.BypassHashVerification = _originalBypass; + PluginHashCalculator.BypassHashVerification = _originalBypassSetting; if (Directory.Exists(_testDataPath)) { diff --git a/src/PluginRegistry/PluginValidator.cs b/src/PluginRegistry/PluginValidator.cs index c2b140c7..bf0ceb03 100644 --- a/src/PluginRegistry/PluginValidator.cs +++ b/src/PluginRegistry/PluginValidator.cs @@ -185,7 +185,7 @@ private static bool RefreshBuiltInPlugins (TrustedPluginConfig config) foreach (var (name, hash) in GetBuiltInPluginHashes()) { - if (!config.PluginHashes.TryGetValue(name, out var persistedHash) || persistedHash != hash) + if (!config.PluginHashes.TryGetValue(name, out var persistedHash) || !string.Equals(persistedHash, hash, StringComparison.OrdinalIgnoreCase)) { config.PluginHashes[name] = hash; changed = true; From 6bafd50763cd287cd8ffefea0434cbaa891f1e07 Mon Sep 17 00:00:00 2001 From: BRUNER Patrick Date: Mon, 19 Oct 2026 14:54:57 +0200 Subject: [PATCH 3/3] Filter RefreshBuiltInPlugins loops explicitly with Where --- src/PluginRegistry/PluginValidator.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/PluginRegistry/PluginValidator.cs b/src/PluginRegistry/PluginValidator.cs index bf0ceb03..9b85922e 100644 --- a/src/PluginRegistry/PluginValidator.cs +++ b/src/PluginRegistry/PluginValidator.cs @@ -174,22 +174,16 @@ private static bool RefreshBuiltInPlugins (TrustedPluginConfig config) { var changed = false; - foreach (var name in _trustedPluginNames) + foreach (var name in _trustedPluginNames.Where(n => !config.PluginNames.Contains(n, StringComparer.OrdinalIgnoreCase))) { - if (!config.PluginNames.Contains(name, StringComparer.OrdinalIgnoreCase)) - { - config.PluginNames.Add(name); - changed = true; - } + config.PluginNames.Add(name); + changed = true; } - foreach (var (name, hash) in GetBuiltInPluginHashes()) + foreach (var (name, hash) in GetBuiltInPluginHashes().Where(e => !config.PluginHashes.TryGetValue(e.Key, out var persistedHash) || !string.Equals(persistedHash, e.Value, StringComparison.OrdinalIgnoreCase))) { - if (!config.PluginHashes.TryGetValue(name, out var persistedHash) || !string.Equals(persistedHash, hash, StringComparison.OrdinalIgnoreCase)) - { - config.PluginHashes[name] = hash; - changed = true; - } + config.PluginHashes[name] = hash; + changed = true; } if (changed)