Refresh built-in plugin hashes when loading trusted-plugins.json (#733)

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.
 /// </summary>
 [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/<Configuration>; plugins are built to bin/<Configuration>/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<TrustedPluginConfig>(File.ReadAllText(Path.Join(_configDir, "trusted-plugins.json")))!;
+    }
+
+    private static TrustedPluginConfig StaleConfig ()
+    {
+        return new TrustedPluginConfig
+        {
+            PluginNames = ["CsvColumnizer.dll", "UserPlugin.dll"],
+            PluginHashes = new Dictionary<string, string>
+            {
+                ["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"));
+        });
+    }
 }

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 ()
         };
     }
 
+    /// <summary>
+    /// 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.
+    /// </summary>
+    /// <returns>True if the configuration was changed</returns>
+    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;
+    }
+
     /// <summary>
     /// Saves trusted plugin configuration to disk.
     /// </summary>

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