diff --git a/src/LogExpert.Tests/Dialogs/SettingsDialogPortableModeTests.cs b/src/LogExpert.Tests/Dialogs/SettingsDialogPortableModeTests.cs new file mode 100644 index 00000000..b23b8a8a --- /dev/null +++ b/src/LogExpert.Tests/Dialogs/SettingsDialogPortableModeTests.cs @@ -0,0 +1,133 @@ +using LogExpert.Core.Config; +using LogExpert.Core.Interfaces; +using LogExpert.Dialogs; + +using Moq; + +using NUnit.Framework; + +using UIStrings = LogExpert.Resources; + +namespace LogExpert.Tests.Dialogs; + +/// +/// Regression tests for issue #658: populating the settings dialog from preferences set the +/// portable-mode checkbox programmatically, which fired the CheckedChanged handler and ran the +/// full activation flow — showing the "copy settings?" question dialog and (re)creating the +/// marker file — every time the dialog was opened while portable mode was active. +/// +[TestFixture] +public class SettingsDialogPortableModeTests +{ + private string _testDataPath = null!; + private string _portableConfigDir = null!; + + private sealed record FillPortableModeResult(bool Finished, Exception? Failure, bool PortableModeAfterFill, string? CheckBoxText); + + [SetUp] + public void SetUp () + { + _testDataPath = Path.Join(Path.GetTempPath(), "LogExpertSettingsDialogTests", Guid.NewGuid().ToString()); + _portableConfigDir = Path.Join(_testDataPath, "configuration"); + } + + [TearDown] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Unit Test")] + public void TearDown () + { + if (Directory.Exists(_testDataPath)) + { + try + { + Directory.Delete(_testDataPath, recursive: true); + } + catch + { + // Ignore cleanup errors + } + } + } + + /// + /// Constructs the dialog and calls FillPortableMode on a disposable STA thread so that a + /// regression (the CheckedChanged flow showing a modal question dialog again) surfaces as + /// a Join timeout instead of hanging the run. + /// + private FillPortableModeResult RunFillPortableMode (bool portableMode) + { + Mock configManager = new(); + _ = configManager.SetupGet(m => m.PortableConfigDir).Returns(_portableConfigDir); + _ = configManager.SetupGet(m => m.PortableModeSettingsFileName).Returns("portableMode.json"); + + Preferences preferences = new() + { + PortableMode = portableMode, + }; + + Exception? failure = null; + var portableModeAfterFill = !portableMode; + string? checkBoxText = null; + + Thread worker = new(() => + { + try + { + using SettingsDialog dialog = new(preferences, null!, 0, configManager.Object); + dialog.FillPortableMode(); + + portableModeAfterFill = dialog.Preferences.PortableMode; + checkBoxText = dialog.Controls.Find("checkBoxPortableMode", searchAllChildren: true).Single().Text; + } + catch (Exception ex) + { + failure = ex; + } + }) + { + IsBackground = true, + }; + worker.SetApartmentState(ApartmentState.STA); + worker.Start(); + var finished = worker.Join(TimeSpan.FromSeconds(20)); + + return new FillPortableModeResult(finished, failure, portableModeAfterFill, checkBoxText); + } + + [Test] + [Description("Issue #658: opening the settings dialog with portable mode active must not prompt or touch files")] + public void FillPortableMode_PortableModeActive_PerformsNoSideEffects () + { + var result = RunFillPortableMode(portableMode: true); + + Assert.That(result.Finished, Is.True, + "FillPortableMode must return without user interaction — a modal question dialog means the portable-mode activation flow ran again"); + Assert.Multiple(() => + { + Assert.That(result.Failure, Is.Null, $"FillPortableMode must not throw, but threw: {result.Failure}"); + Assert.That(Directory.Exists(_portableConfigDir), Is.False, + "Populating the dialog must not create the portable configuration directory or marker file"); + Assert.That(result.PortableModeAfterFill, Is.True, "Preferences must be left untouched"); + Assert.That(result.CheckBoxText, Is.EqualTo(UIStrings.SettingsDialog_UI_DeActivatePortableMode), + "The checkbox label must reflect the active portable mode state"); + }); + } + + [Test] + [Description("Issue #658: populating the dialog with portable mode off must also be side-effect free")] + public void FillPortableMode_PortableModeInactive_PerformsNoSideEffects () + { + var result = RunFillPortableMode(portableMode: false); + + Assert.That(result.Finished, Is.True, + "FillPortableMode must return without user interaction"); + Assert.Multiple(() => + { + Assert.That(result.Failure, Is.Null, $"FillPortableMode must not throw, but threw: {result.Failure}"); + Assert.That(Directory.Exists(_portableConfigDir), Is.False, + "Populating the dialog must not touch the portable configuration directory"); + Assert.That(result.PortableModeAfterFill, Is.False, "Preferences must be left untouched"); + Assert.That(result.CheckBoxText, Is.EqualTo(UIStrings.SettingsDialog_UI_ActivatePortableMode), + "The checkbox label must reflect the inactive portable mode state"); + }); + } +} diff --git a/src/LogExpert.UI/Dialogs/SettingsDialog.cs b/src/LogExpert.UI/Dialogs/SettingsDialog.cs index 580b2a63..8207efdf 100644 --- a/src/LogExpert.UI/Dialogs/SettingsDialog.cs +++ b/src/LogExpert.UI/Dialogs/SettingsDialog.cs @@ -318,9 +318,22 @@ private void FillReaderTypeList () comboBoxReaderType.SelectedItem = Preferences.ReaderType; } - private void FillPortableMode () + internal void FillPortableMode () { + // Detach the handler while syncing the checkbox from preferences: CheckedChanged also + // fires on programmatic changes, and the handler runs the full activation flow + // (question dialog, marker file) which must only happen on a user toggle (issue #658). + checkBoxPortableMode.CheckedChanged -= OnPortableModeCheckedChanged; checkBoxPortableMode.CheckState = Preferences.PortableMode ? CheckState.Checked : CheckState.Unchecked; + SetPortableModeCheckBoxText(); + checkBoxPortableMode.CheckedChanged += OnPortableModeCheckedChanged; + } + + private void SetPortableModeCheckBoxText () + { + checkBoxPortableMode.Text = Preferences.PortableMode + ? Resources.SettingsDialog_UI_DeActivatePortableMode + : Resources.SettingsDialog_UI_ActivatePortableMode; } private void DisplayFontName () @@ -997,7 +1010,7 @@ private void OnPortableModeCheckedChanged (object sender, EventArgs e) } Preferences.PortableMode = true; - checkBoxPortableMode.Text = Resources.SettingsDialog_UI_DeActivatePortableMode; + SetPortableModeCheckBoxText(); // Ask user if they want to copy existing settings var result = MessageBox.Show( @@ -1029,7 +1042,7 @@ private void OnPortableModeCheckedChanged (object sender, EventArgs e) case CheckState.Unchecked: { Preferences.PortableMode = false; - checkBoxPortableMode.Text = Resources.SettingsDialog_UI_ActivatePortableMode; + SetPortableModeCheckBoxText(); // Ask user if they want to move settings back var result = MessageBox.Show( diff --git a/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs b/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs new file mode 100644 index 00000000..655d4d9e --- /dev/null +++ b/src/PluginRegistry.Tests/PluginValidatorConfigPersistenceTests.cs @@ -0,0 +1,135 @@ +using System.Reflection; + +using NUnit.Framework; + +namespace LogExpert.PluginRegistry.Tests; + +/// +/// Regression tests for issue #658: trusted-plugins.json must only ever be written to the +/// directory the validator was configured with. The trust configuration must be loaded +/// lazily (at Initialize or first use), never eagerly at class-load time — an eager load +/// wrote the default file to %APPDATA%/LogExpert before Initialize could point the +/// validator at the portable configuration directory. +/// +/// Limitation: these tests protect the lazy-load mechanism (removing it fails them with a +/// NullReferenceException), but they cannot detect an eager class-load write directly — +/// 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. +/// +[TestFixture] +public class PluginValidatorConfigPersistenceTests +{ + private string _testDataPath = null!; + private string _configDir = null!; + + private string _originalConfigDirectory = null!; + private string _originalConfigPath = null!; + private object? _originalConfig; + + private static FieldInfo GetStaticField (string name) + { + var field = typeof(PluginValidator).GetField(name, BindingFlags.NonPublic | BindingFlags.Static); + Assert.That(field, Is.Not.Null, $"Expected private static field '{name}' on PluginValidator"); + return field!; + } + + [SetUp] + public void SetUp () + { + _testDataPath = Path.Join(Path.GetTempPath(), "LogExpertConfigPersistenceTests", Guid.NewGuid().ToString()); + _configDir = Path.Join(_testDataPath, "activeConfig"); + _ = Directory.CreateDirectory(_configDir); + + _originalConfigDirectory = (string)GetStaticField("_configDirectory").GetValue(null)!; + _originalConfigPath = (string)GetStaticField("_configPath").GetValue(null)!; + _originalConfig = GetStaticField("_trustedPluginConfig").GetValue(null); + } + + [TearDown] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Unit Test")] + public void TearDown () + { + GetStaticField("_configDirectory").SetValue(null, _originalConfigDirectory); + GetStaticField("_configPath").SetValue(null, _originalConfigPath); + GetStaticField("_trustedPluginConfig").SetValue(null, _originalConfig); + + if (Directory.Exists(_testDataPath)) + { + try + { + Directory.Delete(_testDataPath, recursive: true); + } + catch + { + // Ignore cleanup errors + } + } + } + + /// + /// Puts the validator into the state it is in at process start before Initialize has + /// run: no configuration loaded, config path pointing at the (here: sandboxed) default. + /// + private void SimulatePreInitializeState () + { + GetStaticField("_configDirectory").SetValue(null, _configDir); + GetStaticField("_configPath").SetValue(null, Path.Join(_configDir, "trusted-plugins.json")); + GetStaticField("_trustedPluginConfig").SetValue(null, null); + } + + [Test] + [Description("Issue #658: config must be loaded lazily on first use, not eagerly at class load")] + public void ValidatePlugin_WhenConfigNotYetLoaded_LoadsLazilyFromConfiguredDirectory () + { + SimulatePreInitializeState(); + + var pluginPath = Path.Join(_testDataPath, "SomePlugin.dll"); + File.WriteAllText(pluginPath, "not a real assembly"); + + Assert.DoesNotThrow(() => PluginValidator.ValidatePlugin(pluginPath), + "ValidatePlugin must work without an eagerly loaded configuration"); + + Assert.That(File.Exists(Path.Join(_configDir, "trusted-plugins.json")), Is.True, + "Lazy load should create the default configuration in the configured directory"); + } + + [Test] + [Description("Issue #658: AddTrustedPlugin must lazily load and persist to the configured directory")] + public void AddTrustedPlugin_WhenConfigNotYetLoaded_PersistsToConfiguredDirectory () + { + SimulatePreInitializeState(); + + var pluginPath = Path.Join(_testDataPath, "UserPlugin.dll"); + File.WriteAllText(pluginPath, "user plugin content"); + + var result = PluginValidator.AddTrustedPlugin(pluginPath, out var errorMessage); + + Assert.Multiple(() => + { + Assert.That(result, Is.True, $"AddTrustedPlugin should succeed, but failed with: {errorMessage}"); + Assert.That(File.Exists(Path.Join(_configDir, "trusted-plugins.json")), Is.True, + "Trust decision must be persisted to the configured directory"); + }); + } + + [Test] + [Description("Normal mode is unchanged: Initialize creates the default config file in the given directory")] + public void Initialize_WithEmptyDirectory_CreatesDefaultConfigFileThere () + { + SimulatePreInitializeState(); + + var freshDir = Path.Join(_testDataPath, "freshConfig"); + _ = Directory.CreateDirectory(freshDir); + + PluginValidator.Initialize(freshDir); + + Assert.Multiple(() => + { + Assert.That(File.Exists(Path.Join(freshDir, "trusted-plugins.json")), Is.True, + "Initialize should create the default configuration in its directory"); + Assert.That(File.Exists(Path.Join(_configDir, "trusted-plugins.json")), Is.False, + "Nothing may be written to the pre-Initialize default directory"); + }); + } +} diff --git a/src/PluginRegistry/PluginHashGenerator.Generated.cs b/src/PluginRegistry/PluginHashGenerator.Generated.cs index 67fa3bfd..3537bacf 100644 --- a/src/PluginRegistry/PluginHashGenerator.Generated.cs +++ b/src/PluginRegistry/PluginHashGenerator.Generated.cs @@ -10,7 +10,7 @@ public static partial class PluginValidator { /// /// Gets pre-calculated SHA256 hashes for built-in plugins. - /// Generated: 2026-07-13 13:41:01 UTC + /// Generated: 2026-07-20 19:08:11 UTC /// Configuration: Release /// Plugin count: 21 /// @@ -18,27 +18,27 @@ public static Dictionary GetBuiltInPluginHashes() { return new Dictionary(StringComparer.OrdinalIgnoreCase) { - ["AutoColumnizer.dll"] = "5DBCAC9C0B79BA7EA04A612BDB86C14111B53A760D8BA21DA163AA7EEE312F2D", + ["AutoColumnizer.dll"] = "1186873FE8824E2DAE4949AA0CF73FA83CC0F32604337F03C09EEF943AB4F47E", ["BouncyCastle.Cryptography.dll"] = "E5EEAF6D263C493619982FD3638E6135077311D08C961E1FE128F9107D29EBC6", ["BouncyCastle.Cryptography.dll (x86)"] = "E5EEAF6D263C493619982FD3638E6135077311D08C961E1FE128F9107D29EBC6", - ["CsvColumnizer.dll"] = "C15021612BF3B3B88EFEEC58E1CE497455768479ED81A612251D0D27CBB6F448", - ["CsvColumnizer.dll (x86)"] = "C15021612BF3B3B88EFEEC58E1CE497455768479ED81A612251D0D27CBB6F448", - ["DefaultPlugins.dll"] = "FBFB56C2604F7A3B4620135B356B0F3A811CECE98905C3A815ADFCEE6D4DA4B6", - ["FlashIconHighlighter.dll"] = "65FF0C37B7DB4398865BE8F32B8D1EC9C7B03820220A3C06A1099DBBCBF08F99", - ["GlassfishColumnizer.dll"] = "879EA94F28E65D6052B0C605ABBFFC3E11D112E31CBF3710050344D2871A3214", - ["JsonColumnizer.dll"] = "82B4C48BEE20226F23299C8D6AB3C65B2E1FC81553B6FD210FAC59CE3BF813FF", - ["JsonCompactColumnizer.dll"] = "0BAF65806D1B74434E672AB8A1531616768C311657CD1C6B9F0F30D8AD5E8A18", - ["Log4jXmlColumnizer.dll"] = "53B1CD81648DAA3036CC5D9AE017C9E04A0705C900228DF203F2C1D1C812DDCF", - ["LogExpert.Resources.dll"] = "F7F3EDFC4D97789A781B128CD2B3FFBF183079D3D7D7266C85E6E114A054518C", + ["CsvColumnizer.dll"] = "9CACC824A41319679F72195150E72A28BE590DB29A25B21D485A4560A269B399", + ["CsvColumnizer.dll (x86)"] = "9CACC824A41319679F72195150E72A28BE590DB29A25B21D485A4560A269B399", + ["DefaultPlugins.dll"] = "41586EB72ADAE556D858450746BA8121A6D8402AF77993012409C31BA247D835", + ["FlashIconHighlighter.dll"] = "5621E5E57812A0CF71DC9AD2DFDFB504AA843B1157FE93BF22548FD953E178A9", + ["GlassfishColumnizer.dll"] = "2E4F59A130621E7E178D7ADF370D1DD6D64FBC72D287981D660218B8231BA90C", + ["JsonColumnizer.dll"] = "7A8B8E56F21473858001625098A07BA8132B253E4854E4C84C115C8AE8ED8F30", + ["JsonCompactColumnizer.dll"] = "86A555C02580139CB0603BD46A541CF083A4E3ECCE2C48BE9A3D93F498E7D961", + ["Log4jXmlColumnizer.dll"] = "C43D10CF36BE09CC16CA05120EA3BDB70446B451263E69EF27BFE92DD4D20CCD", + ["LogExpert.Resources.dll"] = "48F187EC2B60ED75441CD7DC259A2BB34B512962463572F0504ADA314EBD04D5", ["Microsoft.Extensions.DependencyInjection.Abstractions.dll"] = "67FA4325000DB017DC0C35829B416F024F042D24EFB868BCF17A895EE6500A93", ["Microsoft.Extensions.DependencyInjection.Abstractions.dll (x86)"] = "67FA4325000DB017DC0C35829B416F024F042D24EFB868BCF17A895EE6500A93", ["Microsoft.Extensions.Logging.Abstractions.dll"] = "BB853130F5AFAF335BE7858D661F8212EC653835100F5A4E3AA2C66A4D4F685D", ["Microsoft.Extensions.Logging.Abstractions.dll (x86)"] = "BB853130F5AFAF335BE7858D661F8212EC653835100F5A4E3AA2C66A4D4F685D", - ["RegexColumnizer.dll"] = "90D3155CA4051B9FF23ABA09132F3A4ED2B372FE563CAA3AA68B8ACA18D0C30E", - ["SftpFileSystem.dll"] = "7EFD4BA7657F91CF8C556650F1BEC82B0AE8294D3CED2B83256E4455B44A2954", - ["SftpFileSystem.dll (x86)"] = "9909E6FF04A0903D1A95D849186218055AB9C1CDEFAC2CEAE8208836527C9955", - ["SftpFileSystem.Resources.dll"] = "3E9AA33A653033A0F9B3AC6749B128907969AB11971566ADF8ABA095A72F7F70", - ["SftpFileSystem.Resources.dll (x86)"] = "3E9AA33A653033A0F9B3AC6749B128907969AB11971566ADF8ABA095A72F7F70", + ["RegexColumnizer.dll"] = "6C4601C7544E9978582F7F6CEA706422B24B0C314287FBF7FE737FE18D1B5057", + ["SftpFileSystem.dll"] = "A89597C7C82FEFDC6F72B2583299E360CA6F4F4B9C24B9208C2E3C8E5A27B785", + ["SftpFileSystem.dll (x86)"] = "5A5E6CF4BF48138DE9ED1949A71B2FDAE9B490FD4BA57A1C5259FF7E7F12B4B4", + ["SftpFileSystem.Resources.dll"] = "801CDB49354831FB606357CD910E035832BCFF31AF5E6745DC45885558CF31DC", + ["SftpFileSystem.Resources.dll (x86)"] = "801CDB49354831FB606357CD910E035832BCFF31AF5E6745DC45885558CF31DC", }; } diff --git a/src/PluginRegistry/PluginValidator.cs b/src/PluginRegistry/PluginValidator.cs index 4d7b72d2..00cdd9f9 100644 --- a/src/PluginRegistry/PluginValidator.cs +++ b/src/PluginRegistry/PluginValidator.cs @@ -60,16 +60,6 @@ public static partial class PluginValidator #endregion - #region Constructor - - static PluginValidator () - { - // Load with default path; will be reloaded when Initialize() is called - LoadTrustedPluginConfiguration(); - } - - #endregion - #region Public methods /// @@ -91,6 +81,23 @@ public static void Initialize (string configDirectory) } } + /// + /// Ensures the trusted plugin configuration is loaded. The configuration is loaded + /// lazily on first use (issue #658): an eager load at class-load time would read from + /// and create the file in the default %APPDATA% directory before Initialize() could + /// point the validator at the active (e.g. portable) configuration directory. + /// + private static void EnsureConfigLoaded () + { + lock (_configLock) + { + if (_trustedPluginConfig == null) + { + LoadTrustedPluginConfiguration(); + } + } + } + /// /// Loads trusted plugin configuration from disk. /// @@ -203,6 +210,8 @@ public static bool AddTrustedPlugin (string dllPath, out string errorMessage) var fileName = Path.GetFileName(dllPath); var hash = PluginHashCalculator.CalculateHash(dllPath); + EnsureConfigLoaded(); + lock (_configLock) { if (!_trustedPluginConfig.AllowUserTrustedPlugins) @@ -248,6 +257,8 @@ SecurityException or /// True if removed, false if not found public static bool RemoveTrustedPlugin (string fileName) { + EnsureConfigLoaded(); + lock (_configLock) { var removed = _trustedPluginConfig.PluginNames.Remove(fileName); @@ -298,6 +309,8 @@ public static bool ValidatePlugin (string dllPath, out PluginManifest manifest, try { + EnsureConfigLoaded(); + // 1. Check if file exists if (!File.Exists(dllPath)) {