From 0dbe84cd9f84e73b9da8a6e8a33630c78850b954 Mon Sep 17 00:00:00 2001 From: SysAdminDoc Date: Sun, 14 Jun 2026 02:55:41 -0400 Subject: [PATCH 1/5] fix(safety): protect descendants from recursive ancestor delete A user-protected folder was only guarded at its exact path. Because results are sorted parents-first and deleted recursively, an empty-eligible ancestor of a protected folder was deleted and took the protected child with it (the child was then counted as already-deleted-by-parent, never re-checked). This breached the per-folder protection invariant in both the direct and recycle-batch delete paths (AutoProtectRoot survived only because the root is the topmost item). A directory is now treated as protected when it, or any directory beneath it, is on the protected list (IsProtectedOrAncestorOfProtected), and the protected list is built with an OrdinalIgnoreCase comparer to match Windows path casing. Adds 6 regression tests covering self/ancestor/case-insensitive/prefix-sibling matching. Also: harden the cross-volume Move-to-folder fallback to re-verify file-free and delete the source bottom-up by handle instead of a blind recursive Directory.Delete; and derive the empty-files recycle batch outcome from !File.Exists rather than the sink's positional results. --- CHANGELOG.md | 3 ++ RED.Tests/DeletionWorkerTests.cs | 67 ++++++++++++++++++++++++++++++++ RED/RedLib/Core.cs | 5 ++- RED/RedLib/DeletionWorker.cs | 52 ++++++++++++++++++++----- RED/RedLib/RuntimeData.cs | 4 +- RED/RedLib/SystemFunctions.cs | 9 ++++- RED/UI/MainWindow.cs | 2 +- 7 files changed, 125 insertions(+), 17 deletions(-) create mode 100644 RED.Tests/DeletionWorkerTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 29a2c81..4f1200b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,8 +22,11 @@ ### Reliability - Derive batch-recycle outcomes (success/fail and the undo manifest) authoritatively from `Directory.Exists` rather than the shell sink's submission-order fallback. `IFileOperation` does not guarantee one `PostDeleteItem` per `DeleteItem` in order, so a coalesced/skipped callback could previously record an undo entry against the wrong path; the filesystem is now the ground truth. +- Apply the same ground-truth rule to the empty-files recycle batch: each file's success and undo entry now come from `!File.Exists` rather than the sink's positional result list, so a coalesced callback can no longer log or record the wrong file as deleted. ### Security & data safety +- Never delete a directory that contains a user-protected descendant. Protecting a folder only guarded that exact path, so an empty-eligible **ancestor** would still be deleted recursively and take the protected child with it (in both the direct and recycle-batch delete paths). A directory is now treated as protected when it, or anything beneath it, is on the protected list, and the protected-folder list is matched case-insensitively to follow Windows path semantics. +- Harden the cross-volume Move-to-folder fallback: after copying the directory to the other volume, re-verify it is file-free and remove the source bottom-up by handle (which refuses a non-empty directory) instead of a blind recursive `Directory.Delete`, so content that appears after the copy can never be destroyed. - Clamp the C-style (`SpecialFormatters`) printf width and precision to a bounded ceiling so a crafted format such as `%9999999d` can no longer drive a multi-megabyte allocation (memory-amplification DoS for callers that opt into the format callback). - Guard `Translator.RegisterTranslationsByCulture` against a malformed caller-supplied search pattern: a bad `string.Format` pattern is now skipped instead of throwing an uncaught `FormatException`. - Stop truncating the live run log when rotation cannot move it (e.g. the log is held open). The existing log is now preserved and appended to, keeping it intact as a forensic/undo aid. diff --git a/RED.Tests/DeletionWorkerTests.cs b/RED.Tests/DeletionWorkerTests.cs new file mode 100644 index 0000000..1e01a5e --- /dev/null +++ b/RED.Tests/DeletionWorkerTests.cs @@ -0,0 +1,67 @@ +using System; +using System.Collections.Generic; +using Xunit; + +namespace RED +{ + // Locks in the protected-folder invariant: a directory must not be deleted when + // it (or any ancestor) is the parent of a user-protected folder. Regression guard + // for the bug where a protected child was destroyed by an empty-eligible ancestor's + // recursive delete because only the child's own path was checked. + public class DeletionWorkerTests + { + private static HashSet Set(params string[] paths) + { + return new HashSet(paths, StringComparer.OrdinalIgnoreCase); + } + + [Fact] + public void Protect_Self_IsProtected() + { + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b\keep", Set(@"C:\a\b\keep"))); + } + + [Fact] + public void Protect_Ancestor_OfProtected_IsProtected() + { + var p = Set(@"C:\a\b\keep"); + // Every ancestor of a protected folder must itself be treated as protected, + // or a recursive delete of the ancestor would destroy the protected child. + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b", p)); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a", p)); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b\", p)); // trailing separator + } + + [Fact] + public void Protect_IsCaseInsensitive() + { + var p = Set(@"C:\Data\Keep"); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"c:\data", p)); + Assert.True(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\DATA\KEEP", p)); + } + + [Fact] + public void Sibling_And_Unrelated_AreNotProtected() + { + var p = Set(@"C:\a\b\keep"); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\b\other", p)); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\x", p)); + } + + [Fact] + public void PrefixSibling_IsNotMistakenForAncestor() + { + // "C:\a\bc" shares a string prefix with "C:\a\b\keep" but is NOT an ancestor; + // the path-separator boundary must prevent a false protection match. + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a\bc", Set(@"C:\a\b\keep"))); + } + + [Fact] + public void EmptyOrNull_AreNotProtected() + { + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(@"C:\a", new HashSet())); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(null, Set(@"C:\a"))); + Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected("", Set(@"C:\a"))); + } + } +} diff --git a/RED/RedLib/Core.cs b/RED/RedLib/Core.cs index e3bb6dd..10c74a1 100644 --- a/RED/RedLib/Core.cs +++ b/RED/RedLib/Core.cs @@ -48,8 +48,9 @@ public void SearchingForEmptyDirectories() { CurrentProcessStep = WorkflowSteps.StartSearchingForEmptyDirs; - // Rest folder list - RunData.ProtectedFolderList = new Dictionary(); + // Reset protected-folder list. Case-insensitive to match Windows path + // semantics (a protect entry must match the scan result regardless of casing). + RunData.ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); // Start async empty directory search worker SearchEmptyFoldersWorker = new FindEmptyDirectoryWorker(); diff --git a/RED/RedLib/DeletionWorker.cs b/RED/RedLib/DeletionWorker.cs index d9319f5..7fe68cb 100644 --- a/RED/RedLib/DeletionWorker.cs +++ b/RED/RedLib/DeletionWorker.cs @@ -34,6 +34,29 @@ public class DeletionWorker : BackgroundWorker private readonly System.Collections.Generic.HashSet deletedParents = new System.Collections.Generic.HashSet(StringComparer.OrdinalIgnoreCase); + /// + /// True when is itself user-protected OR is an + /// ancestor of any protected path. A directory that merely contains a protected + /// descendant must never be deleted: a recursive delete of the ancestor destroys + /// the protected child, and the per-item protection check on the child's own + /// path is then bypassed (the child is reported as already-deleted-by-parent). + /// Comparison is case-insensitive to match Windows path semantics and the + /// OrdinalIgnoreCase ProtectedFolderList. + /// + internal static bool IsProtectedOrAncestorOfProtected(string fullPath, System.Collections.Generic.ICollection protectedPaths) + { + if (string.IsNullOrEmpty(fullPath) || protectedPaths == null || protectedPaths.Count == 0) + return false; + string prefix = fullPath.TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; + foreach (string p in protectedPaths) + { + if (string.IsNullOrEmpty(p)) continue; + if (string.Equals(p, fullPath, StringComparison.OrdinalIgnoreCase)) return true; + if (p.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) return true; + } + return false; + } + public DeletionWorker() { WorkerReportsProgress = true; @@ -114,8 +137,9 @@ protected override void OnDoWork(DoWorkEventArgs e) MovedTo = null }); } - // Do not delete one time protected folders - else if (!this.Data.ProtectedFolderList.ContainsKey(scanResult.FullPath)) + // Never delete a protected folder, nor any directory that contains a + // protected descendant (a recursive delete would take the child with it). + else if (!IsProtectedOrAncestorOfProtected(scanResult.FullPath, this.Data.ProtectedFolderList.Keys)) { try { @@ -155,6 +179,7 @@ protected override void OnDoWork(DoWorkEventArgs e) else { status = DirectoryDeletionStatusTypes.Protected; + this.ProtectedCount++; } this.ReportProgress(1, new DeleteProcessUpdateEventArgs(this.ListPos, scanResult, status, this.Data.ScanResults.Count)); @@ -218,8 +243,9 @@ private void BatchRecycleRun(DoWorkEventArgs e) Match.RedScanResultItem scanResult = this.Data.ScanResults[pos]; - if (this.Data.ProtectedFolderList.ContainsKey(scanResult.FullPath)) + if (IsProtectedOrAncestorOfProtected(scanResult.FullPath, this.Data.ProtectedFolderList.Keys)) { + this.ProtectedCount++; this.ReportProgress(1, new DeleteProcessUpdateEventArgs(pos, scanResult, DirectoryDeletionStatusTypes.Protected, total)); continue; } @@ -409,20 +435,26 @@ private void DeleteEmptyFilesBatch(List files) bool showErrors = this.Data.DeleteMode == DeleteModes.RecycleBinShowErrors && !NotBob.Config.ConfigAssist.SilentMode; bool showConfirm = this.Data.DeleteMode == DeleteModes.RecycleBinWithQuestion && !NotBob.Config.ConfigAssist.SilentMode; - List results = RecycleBinOperation.RecycleBatch( + RecycleBinOperation.RecycleBatch( paths, showConfirm, showErrors, () => CancellationPending, null); - for (int i = 0; i < results.Count; i++) + // Derive each file's outcome from the filesystem, not the sink's positional + // result list: IFileOperation does not guarantee one PostDeleteItem per + // DeleteItem in submission order, so trusting results[i] can attribute a + // success/failure (and the undo entry) to the wrong file. A recycled file is + // gone from its origin, so !File.Exists is the ground truth (matches the + // directory batch path). + foreach (string filePath in paths) { - RecycleBinOperation.ItemResult r = results[i]; - if (r.Succeeded) + bool deleted = !File.Exists(filePath); + if (deleted) { - this.Data.AddLogMessage(TXT.Translate("Successfully deleted empty file: {0}", RedAssist.DQuote(r.Path))); + this.Data.AddLogMessage(TXT.Translate("Successfully deleted empty file: {0}", RedAssist.DQuote(filePath))); this.DeletedFileCount++; undoEntries.Add(new UndoManager.ManifestEntry { - Path = r.Path, + Path = filePath, Mode = this.Data.DeleteMode.ToString(), MovedTo = null, IsFile = true @@ -430,7 +462,7 @@ private void DeleteEmptyFilesBatch(List files) } else { - this.Data.AddLogMessage(TXT.Translate("Failed to delete empty file: {0} - HRESULT 0x{1:X8}", RedAssist.DQuote(r.Path), r.HResult)); + this.Data.AddLogMessage(TXT.Translate("Failed to delete empty file: {0}", RedAssist.DQuote(filePath))); this.FailedFileCount++; } } diff --git a/RED/RedLib/RuntimeData.cs b/RED/RedLib/RuntimeData.cs index 3cba2fa..e06e89f 100644 --- a/RED/RedLib/RuntimeData.cs +++ b/RED/RedLib/RuntimeData.cs @@ -15,7 +15,7 @@ public class RuntimeData : IDisposable public RuntimeData() { this.LogMessages = new StringBuilder(); - this.ProtectedFolderList = new Dictionary(); + this.ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); this.ScanResults = new RedScanResultItemList(); try @@ -108,7 +108,7 @@ private static bool RotateLogIfNeeded(string logPath, out string rotatedPath) public int InfiniteLoopDetectionCount { get; set; } public StringBuilder LogMessages = null; - public Dictionary ProtectedFolderList = new Dictionary(); + public Dictionary ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); public bool HideIgnoredDirectories { get; set; } public bool RespectGitIgnore { get; set; } diff --git a/RED/RedLib/SystemFunctions.cs b/RED/RedLib/SystemFunctions.cs index 1e8c130..f57ad0b 100644 --- a/RED/RedLib/SystemFunctions.cs +++ b/RED/RedLib/SystemFunctions.cs @@ -509,9 +509,14 @@ public static void SecureDeleteDirectory(string path, DeleteModes deleteMode, ou { VerifySubtreeHasNoFiles(path); } - // Directory.Move cannot cross volumes — replicate, then remove the source + // Directory.Move cannot cross volumes — replicate, then remove the + // source. Re-verify file-free one last time and delete bottom-up by + // handle (which refuses a non-empty directory) instead of a blind + // recursive Directory.Delete that would destroy anything that appeared + // after the copy. CopyDirectoryRecursive(path, destPath); - Directory.Delete(path, true); + VerifySubtreeHasNoFiles(path); + DeleteEmptySubtreeByHandle(path); } movedToDestination = destPath; return; diff --git a/RED/UI/MainWindow.cs b/RED/UI/MainWindow.cs index bf961c2..0c41ce8 100644 --- a/RED/UI/MainWindow.cs +++ b/RED/UI/MainWindow.cs @@ -1787,7 +1787,7 @@ private void mnuItemImportDryRunResults_Click(object sender, EventArgs e) } } - RunData.ProtectedFolderList = new Dictionary(); + RunData.ProtectedFolderList = new Dictionary(StringComparer.OrdinalIgnoreCase); RunData.StartFolder = imported.Roots.Count == 1 ? imported.Roots[0].RootDirectory : null; var directoryRoots = new List(); From eb5e5914f9870ec021b05ff33e26c052e86ea6c2 Mon Sep 17 00:00:00 2001 From: SysAdminDoc Date: Sun, 14 Jun 2026 02:59:12 -0400 Subject: [PATCH 2/5] fix(cli): validate saved profiles and harden config/event-log paths - -saveprofile now rejects an unknown -mode, or -mode move without an absolute -moveto, with exit 1 instead of persisting a profile that fails every later -profile run. Profile names are trimmed and capped at 128 chars with control characters rejected (they corrupt -listprofiles output). - Config load restores any sub-object that a crafted file nil'd out, so the dirty-state check (incl. ConfigLoad's finally) can no longer NullReference and crash a headless run with a non-deterministic exit code. - The -eventlog summary now sanitizes folder names (bidi/zero-width/control) like the console and log lines already do. --- CHANGELOG.md | 3 +++ RED/Config/Config.cs | 16 ++++++++++++++++ RED/Config/ConfigAssist.cs | 4 ++++ RED/Program.cs | 19 +++++++++++++++++++ RED/RedLib/EventLogWriter.cs | 4 ++++ RED/RedLib/ProfileStore.cs | 16 ++++++++++++++++ 6 files changed, 62 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f1200b..f70e22f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Add **Import saved dry-run results...** to the modern WPF shell's Extras menu: load a saved `.json`/`.ndjson`/`.csv`/`.txt` dry-run and review the records in the results list (review/export only; re-scan to delete, and the engine re-checks every directory before acting). Review of saved runs no longer requires `-classic`. ### CLI +- Validate `-saveprofile` options before writing: an unknown `-mode`, or `-mode move` without an absolute `-moveto`, is now rejected with exit code 1 instead of silently saving a profile that fails every later `-profile` run. Profile names are trimmed and rejected if they exceed 128 characters or contain control characters (which would corrupt the `-listprofiles` output). - Add saved profiles: `-saveprofile ` stores the current options (paths, mode, empty-files, age/depth, gitignore/MFT/hidden/system toggles) as a named profile; `-profile ` runs it (command-line options still override); `-listprofiles` lists them. A scheduled task can now reference `-profile nightly` instead of a long argument list. Profiles live in a dedicated `RED+.profiles.json`, separate from the XML config. - Headless runs now print a "Run complete" summary line with total empty directories, empty files, deleted, failed, and wall-clock duration. The `-json` `meta` record (now schema 3) carries the same totals plus `elapsedMs`. (The modern GUI already shows the current directory being scanned in its status strip.) @@ -30,6 +31,8 @@ - Clamp the C-style (`SpecialFormatters`) printf width and precision to a bounded ceiling so a crafted format such as `%9999999d` can no longer drive a multi-megabyte allocation (memory-amplification DoS for callers that opt into the format callback). - Guard `Translator.RegisterTranslationsByCulture` against a malformed caller-supplied search pattern: a bad `string.Format` pattern is now skipped instead of throwing an uncaught `FormatException`. - Stop truncating the live run log when rotation cannot move it (e.g. the log is held open). The existing log is now preserved and appended to, keeping it intact as a forensic/undo aid. +- Restore a default sub-object after loading a config whose child element was explicitly nil'd (e.g. a hand-edited ``), which would otherwise NullReference inside the dirty-state check — including in the load's `finally`, crashing a headless run with a non-deterministic exit code. +- Run the scanned paths through the same bidi/zero-width/control-character sanitizer before writing the `-eventlog` summary, so a crafted folder name cannot reorder or corrupt the Windows Event Viewer entry. - Make the MFT/USN turbo scanner fail closed on an incomplete enumeration. The volume walk now treats any termination other than the EOF sentinel as truncated, and rejects a result set in which a directory referenced as a parent is missing its own record (the signature of a dropped USN record). In either case the scan falls back to the standard recursive walker instead of risking a non-empty directory being reported empty because its children were dropped. ### Developer / CI diff --git a/RED/Config/Config.cs b/RED/Config/Config.cs index a94555b..722ad9f 100644 --- a/RED/Config/Config.cs +++ b/RED/Config/Config.cs @@ -57,6 +57,22 @@ internal void SetToDefaults() UI.SetToDefaults(); } + /// + /// Replaces any null sub-object with a fresh default. A hand-crafted config that + /// nils a child (e.g. <Options xsi:nil="true"/>) would otherwise + /// deserialize with a null member and NullReference inside the DataIsDirty + /// getter/setter — including in ConfigLoad's finally, where it could crash a + /// headless run with a non-deterministic exit code. + /// + internal void EnsureSubObjects() + { + if (Options == null) Options = new ConfigOptions(); + if (Filters == null) Filters = new ConfigFilters(); + if (UI == null) UI = new ConfigUI(); + if (Volatile == null) Volatile = new ConfigVolatile(); + if (Runtime == null) Runtime = new ConfigRuntime(); + } + public void PopulateRuntime(string configFilename, string executableName, string productName, string productVersion) { Runtime.CreatedBy = string.Format("{0} {1}", productName, productVersion); diff --git a/RED/Config/ConfigAssist.cs b/RED/Config/ConfigAssist.cs index 0a7e7d7..32fcefd 100644 --- a/RED/Config/ConfigAssist.cs +++ b/RED/Config/ConfigAssist.cs @@ -39,6 +39,10 @@ internal static void ConfigLoad(ref RedConfiguration config, string appName) { config = ConfigAssist.Load(filename); + // A crafted/corrupt file can deserialize with a nil child object; + // restore any missing sub-object before anything dereferences it. + config?.EnsureSubObjects(); + // Does the config file redirect to another location? if (!string.IsNullOrWhiteSpace(config.RedirectTo)) { diff --git a/RED/Program.cs b/RED/Program.cs index 14a281b..b015ee5 100644 --- a/RED/Program.cs +++ b/RED/Program.cs @@ -343,6 +343,25 @@ private static void Main() if (saveProfileName != null) { + // Validate the options now so the saved profile is guaranteed runnable, + // instead of silently saving a profile that fails every later -profile run. + if (modeOverride != null && !ModeAliases.ContainsKey(modeOverride)) + { + if (!quiet) Console.Error.WriteLine("Error: unknown -mode '" + modeOverride + "' (use recycle|direct|move|simulate)"); + Environment.ExitCode = 1; + return; + } + if (modeOverride != null && ModeAliases[modeOverride] == DeleteModes.MoveToFolder) + { + string expandedTarget = string.IsNullOrWhiteSpace(moveTarget) ? null : Environment.ExpandEnvironmentVariables(moveTarget); + if (string.IsNullOrWhiteSpace(expandedTarget) || !System.IO.Path.IsPathRooted(expandedTarget)) + { + if (!quiet) Console.Error.WriteLine("Error: -mode move requires an absolute -moveto "); + Environment.ExitCode = 1; + return; + } + } + var prof = new RedProfile { Name = saveProfileName, diff --git a/RED/RedLib/EventLogWriter.cs b/RED/RedLib/EventLogWriter.cs index 7c39536..21a8be0 100644 --- a/RED/RedLib/EventLogWriter.cs +++ b/RED/RedLib/EventLogWriter.cs @@ -18,6 +18,10 @@ internal static void WriteRunSummary(string scanPaths, string deleteMode, EventLog.CreateEventSource(SourceName, LogName); } + // Folder names reach the Event Viewer here; neutralize bidi/zero-width/ + // control characters as the console and log lines already do. + scanPaths = RED.Helper.RedAssist.SanitizeDisplay(scanPaths); + string message = string.Format( "RED++ scan completed.\r\n" + "Paths: {0}\r\n" + diff --git a/RED/RedLib/ProfileStore.cs b/RED/RedLib/ProfileStore.cs index 5e1e2b3..3c1e4f4 100644 --- a/RED/RedLib/ProfileStore.cs +++ b/RED/RedLib/ProfileStore.cs @@ -78,6 +78,22 @@ public static bool Save(RedProfile profile, out string error) error = "Profile name is required."; return false; } + profile.Name = profile.Name.Trim(); + // A control character (tab/newline) would corrupt the tab-delimited + // -listprofiles output; an unbounded name bloats the store. + if (profile.Name.Length > 128) + { + error = "Profile name must be 128 characters or fewer."; + return false; + } + foreach (char c in profile.Name) + { + if (char.IsControl(c)) + { + error = "Profile name must not contain control characters."; + return false; + } + } try { List all = LoadAll(); From a9a7280802576858e415e3ddcd9431f6991b0755 Mon Sep 17 00:00:00 2001 From: SysAdminDoc Date: Sun, 14 Jun 2026 03:03:54 -0400 Subject: [PATCH 3/5] fix(filters): scope per-directory gitignore names and bound regex matching - A bare-name rule in a per-directory .gitignore (e.g. `dist`) was applied to every directory of that name tree-wide instead of only its own subtree, so unrelated directories were wrongly reported ignored/skipped in the dry-run. Name patterns now honor their scope like path patterns do (Git semantics). - User filter regexes (RegExName/RegExPath) had no match timeout, so a catastrophic-backtracking pattern could hang the scan thread. They now carry a one-second timeout and a timed-out match is treated as no-match. Adds a scoped-gitignore test and a ReDoS-timeout test. --- CHANGELOG.md | 2 ++ RED.Tests/GitIgnoreTests.cs | 18 ++++++++++++++++++ RED.Tests/RedMatchTests.cs | 17 +++++++++++++++++ RED/RedLib/GitIgnoreParser.cs | 9 +++++++++ RED/RedMatch/RedMatchItem.cs | 17 +++++++++++++++-- RED/RedMatch/RedMatchItemList.cs | 4 ++-- 6 files changed, 63 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f70e22f..1cdfdad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,10 +24,12 @@ ### Reliability - Derive batch-recycle outcomes (success/fail and the undo manifest) authoritatively from `Directory.Exists` rather than the shell sink's submission-order fallback. `IFileOperation` does not guarantee one `PostDeleteItem` per `DeleteItem` in order, so a coalesced/skipped callback could previously record an undo entry against the wrong path; the filesystem is now the ground truth. - Apply the same ground-truth rule to the empty-files recycle batch: each file's success and undo entry now come from `!File.Exists` rather than the sink's positional result list, so a coalesced callback can no longer log or record the wrong file as deleted. +- Scope a per-directory `.gitignore`'s bare-name rule (e.g. `dist`) to that directory's own subtree, matching Git semantics. Previously such a rule was applied tree-wide, so directories of that name elsewhere were wrongly reported as ignored (and skipped) in the dry-run the user reviews before deleting. ### Security & data safety - Never delete a directory that contains a user-protected descendant. Protecting a folder only guarded that exact path, so an empty-eligible **ancestor** would still be deleted recursively and take the protected child with it (in both the direct and recycle-batch delete paths). A directory is now treated as protected when it, or anything beneath it, is on the protected list, and the protected-folder list is matched case-insensitively to follow Windows path semantics. - Harden the cross-volume Move-to-folder fallback: after copying the directory to the other volume, re-verify it is file-free and remove the source bottom-up by handle (which refuses a non-empty directory) instead of a blind recursive `Directory.Delete`, so content that appears after the copy can never be destroyed. +- Bound user-supplied filter regexes (`RegExName`/`RegExPath`) with a one-second match timeout, so a catastrophic-backtracking pattern (e.g. `(a+)+$`) can no longer hang the scan thread; a timed-out match is treated as no-match. - Clamp the C-style (`SpecialFormatters`) printf width and precision to a bounded ceiling so a crafted format such as `%9999999d` can no longer drive a multi-megabyte allocation (memory-amplification DoS for callers that opt into the format callback). - Guard `Translator.RegisterTranslationsByCulture` against a malformed caller-supplied search pattern: a bad `string.Format` pattern is now skipped instead of throwing an uncaught `FormatException`. - Stop truncating the live run log when rotation cannot move it (e.g. the log is held open). The existing log is now preserved and appended to, keeping it intact as a forensic/undo aid. diff --git a/RED.Tests/GitIgnoreTests.cs b/RED.Tests/GitIgnoreTests.cs index 72393ef..1ca748d 100644 --- a/RED.Tests/GitIgnoreTests.cs +++ b/RED.Tests/GitIgnoreTests.cs @@ -71,5 +71,23 @@ public void CommentsAndBlankLines_Ignored() Assert.True(p.IsIgnored("cache", "cache")); Assert.False(p.IsIgnored("a", "a")); } + + [Fact] + public void NamePattern_InSubdirGitignore_IsScopedToThatSubtree() + { + // A bare-name rule in a per-directory .gitignore must only affect that + // directory's subtree, not every directory of that name elsewhere (Git + // per-directory scope). Regression guard: previously it applied tree-wide. + string sub = Path.Combine(_root, "sub"); + Directory.CreateDirectory(sub); + File.WriteAllText(Path.Combine(sub, ".gitignore"), "dist\n"); + + var atSub = RED.GitIgnoreParser.LoadFromAncestors(_root).ExtendForDirectory(sub, _root); + + Assert.True(atSub.IsIgnored("dist", "sub/dist")); // in scope -> ignored + Assert.True(atSub.IsIgnored("dist", "sub/a/dist")); // deeper in scope -> ignored + Assert.False(atSub.IsIgnored("dist", "other/dist")); // different subtree -> NOT ignored + Assert.False(atSub.IsIgnored("dist", "dist")); // at root -> NOT ignored + } } } diff --git a/RED.Tests/RedMatchTests.cs b/RED.Tests/RedMatchTests.cs index e5e0d06..76adf1f 100644 --- a/RED.Tests/RedMatchTests.cs +++ b/RED.Tests/RedMatchTests.cs @@ -1,5 +1,7 @@ using System.Collections.Generic; +using System.Diagnostics; using System.IO; +using System.Text.RegularExpressions; using RED.Match; using Xunit; @@ -39,6 +41,21 @@ public void RegexNameRule_MatchesCaseInsensitive() Assert.False(list.IsOnList(new DirectoryInfo(@"C:\x\temp"))); } + [Fact] + public void RegexRule_PathologicalPattern_DoesNotHangAndYieldsNoMatch() + { + // A catastrophic-backtracking pattern against a long name must hit the + // bounded match timeout and resolve to "no match" instead of wedging the + // scan thread. Regression guard for the missing regex matchTimeout. + var list = BuildDirList("+|RN|/(a+)+$/"); + var dir = new DirectoryInfo(@"C:\x\" + new string('a', 44) + "!"); + var sw = Stopwatch.StartNew(); + bool hit = list.IsOnList(dir); + sw.Stop(); + Assert.False(hit); + Assert.True(sw.ElapsedMilliseconds < 5000, "regex match should time out fast, took " + sw.ElapsedMilliseconds + "ms"); + } + [Fact] public void WildcardCodelessRule_BecomesNameRegex() { diff --git a/RED/RedLib/GitIgnoreParser.cs b/RED/RedLib/GitIgnoreParser.cs index 1ea1ecf..4405384 100644 --- a/RED/RedLib/GitIgnoreParser.cs +++ b/RED/RedLib/GitIgnoreParser.cs @@ -173,6 +173,15 @@ public bool IsIgnored(string name, string relativePath) } else { + // A per-directory .gitignore's bare-name rule is scoped to that + // directory's own subtree, exactly like a path pattern. Without this + // guard a deep ".gitignore" line such as `dist` would wrongly ignore + // every `dist` folder tree-wide (siblings and ancestors included). + if (!string.IsNullOrEmpty(rule.ScopeDir) + && !pathToCheck.StartsWith(rule.ScopeDir + "/", StringComparison.OrdinalIgnoreCase)) + { + continue; + } text = name; } diff --git a/RED/RedMatch/RedMatchItem.cs b/RED/RedMatch/RedMatchItem.cs index 41d02ef..9e96bc2 100644 --- a/RED/RedMatch/RedMatchItem.cs +++ b/RED/RedMatch/RedMatchItem.cs @@ -32,8 +32,10 @@ public void Populate(RedMatchMethodType matchMethod, string matchText, bool enab if (matchMethod == RedMatchMethodType.RegExName || matchMethod == RedMatchMethodType.RegExPath) { // All other match methods compare case-insensitively (names are - // lowercased before matching) — regex rules must behave the same - RegEx = new Regex(ExpandRegEx(matchText), RegexOptions.IgnoreCase); + // lowercased before matching) — regex rules must behave the same. + // Bound backtracking so a pathological user pattern (e.g. "(a+)+$") + // cannot wedge the scan thread; a timeout is treated as no-match. + RegEx = new Regex(ExpandRegEx(matchText), RegexOptions.IgnoreCase, System.TimeSpan.FromSeconds(1)); } MatchText = matchText.Trim(); MatchTextToCompare = MatchText.ToLowerInvariant(); @@ -66,6 +68,17 @@ private string ExpandRegEx(string text) return respx; } + /// + /// Bounded regex match: a user pattern that exhausts its match timeout is + /// treated as no-match rather than throwing out of the hot scan loop. + /// + internal static bool SafeIsMatch(Regex regex, string input) + { + if (regex == null) return false; + try { return regex.IsMatch(input); } + catch (RegexMatchTimeoutException) { return false; } + } + internal static string FormatToString(RedMatchItem item) { return FormatToString(item.Enabled, item.MatchMethod, item.MatchText); diff --git a/RED/RedMatch/RedMatchItemList.cs b/RED/RedMatch/RedMatchItemList.cs index 93cecd9..82d0c56 100644 --- a/RED/RedMatch/RedMatchItemList.cs +++ b/RED/RedMatch/RedMatchItemList.cs @@ -195,11 +195,11 @@ private bool IsOnList(string nameToCheck, string pathToCheck, long filesize, boo hit = textToCheck.StartsWith(matchItem.MatchTextToCompare); break; case RedMatchMethodType.RegExName: - hit = matchItem.RegEx.IsMatch(nameToCheck); + hit = RedMatchItem.SafeIsMatch(matchItem.RegEx, nameToCheck); break; case RedMatchMethodType.RegExPath: if (FilterType == RedMatchFilterType.Directory) - hit = matchItem.RegEx.IsMatch(pathToCheck); + hit = RedMatchItem.SafeIsMatch(matchItem.RegEx, pathToCheck); break; case RedMatchMethodType.NameExactWithPath: if (FilterType == RedMatchFilterType.Directory) From 8905eb49add02a58e5b49d928b8a48f558aa7c97 Mon Sep 17 00:00:00 2001 From: SysAdminDoc Date: Sun, 14 Jun 2026 03:14:24 -0400 Subject: [PATCH 4/5] style(wpf): color-code results, add focus rings, fix dry-run copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Premium/accessibility pass on the default WPF shell: - The review list's computed status color was never bound to anything, so the destructive list had no color coding despite the legend promising it. Status is now a colored word (eligible=red, kept=muted, deleted=green, protected=blue, failed=amber) — color reinforces the word, never alone. - Text box, deletion-mode dropdown, filter lists, and result list now show a visible BlueLight keyboard-focus ring (were WPF's invisible dotted default). - A dry run no longer claims files were "changed": it reports "would be removed. Nothing was changed.", and completion copy matches the chosen mode. Canceled vs error-stopped runs read distinctly. - Legible red for eligible status text, comfortable row height, green/amber status indicator with a screen-reader-friendly name, a positive "no empty directories found" state, and one-off colors/radii folded onto the palette. --- CHANGELOG.md | 5 + RED/UI/Wpf/ModernMainWindow.cs | 173 ++++++++++++++++++++++++++++----- 2 files changed, 156 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cdfdad..57ec9ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ - Drop the advertised "Enter to scan, Del to delete selected" keyboard shortcuts from the feature list: the default modern WPF shell is click-only by design, matching the project's no-keyboard-shortcuts convention. ### Modern UI +- Colour-code the review list: each result's Status is shown as a coloured word (eligible = red, kept = muted, deleted = green, protected = blue, failed = amber), so the destructive list now reflects the legend instead of leaving every row the same colour. The colour reinforces the status word and is never the only signal. +- Give the path box, deletion-mode dropdown, filter lists, and result list a visible keyboard-focus ring (they previously showed only WPF's near-invisible dotted default on the dark surface) — WCAG 2.4.7. +- Report a dry run honestly: it now says "… would be removed. Nothing was changed." instead of claiming directories and files were "changed", and the completion line matches the chosen mode (deleted / recycled / moved). Distinguish a user **Canceled** run from one **Stopped after an error**. +- Use a legible red for the "eligible" status text, give result rows a comfortable minimum height, raise the idle status indicator to green / working to amber, and expose the plain state word ("Ready"/"Working") to screen readers instead of the decorative bullet. +- Add a positive "no empty directories found" state for a completed scan with nothing to remove, and normalize one-off button colours and corner radii onto the shared palette. - Add **Restore deletion** to the modern WPF shell's Extras menu: pick any kept undo manifest (newest first, with timestamp, mode, and item count) and restore it on a background thread with live status. Recovery no longer requires `-classic`. - Add **Import saved dry-run results...** to the modern WPF shell's Extras menu: load a saved `.json`/`.ndjson`/`.csv`/`.txt` dry-run and review the records in the results list (review/export only; re-scan to delete, and the engine re-checks every directory before acting). Review of saved runs no longer requires `-classic`. diff --git a/RED/UI/Wpf/ModernMainWindow.cs b/RED/UI/Wpf/ModernMainWindow.cs index 8cf82c6..d7ba75a 100644 --- a/RED/UI/Wpf/ModernMainWindow.cs +++ b/RED/UI/Wpf/ModernMainWindow.cs @@ -45,6 +45,11 @@ internal sealed class ModernMainWindow : Window private Grid contentHost; private Border resultSurface; private Grid emptyState; + private System.Windows.Shapes.Path emptyIcon; + private TextBlock emptyTitle; + private TextBlock emptySubtitle; + private StackPanel emptyTrust; + private bool hasScanned; private ListView resultsList; private WpfTextBox pathBox; private WpfButton scanButton; @@ -91,8 +96,17 @@ internal sealed class ModernMainWindow : Window private static readonly Brush Blue = BrushFrom("#2f6df2"); private static readonly Brush BlueLight = BrushFrom("#7aa8ff"); private static readonly Brush Red = BrushFrom("#dc3548"); + // A brighter red than the legend swatch, for status text that must stay legible + // on the dark result surface (the #dc3548 swatch dips below AA at body size). + private static readonly Brush RedText = BrushFrom("#ff7a86"); private static readonly Brush Green = BrushFrom("#67d16f"); + private static readonly Brush Amber = BrushFrom("#ffca55"); private static readonly Brush Pink = BrushFrom("#f17aa5"); + + // Keyboard-focus ring for the non-button controls (text box, combo, lists), + // which otherwise show only WPF's near-invisible dotted default on this dark + // surface. Matches the BlueLight ring the buttons already use (WCAG 2.4.7). + private static readonly Style FocusVisual = CreateFocusVisual(); private static readonly Geometry IconSearch = Glyph("M17,8 A9,9 0 1 1 17,26 A9,9 0 1 1 17,8 M24,24 L32,32"); private static readonly Geometry IconSettings = Glyph("M18,5 L20.6,9.7 L26,10.8 L22.4,15 L24.1,20.3 L18.8,19 L15.2,23.2 L12.6,18.5 L7.2,17.4 L10.8,13.2 L9.1,7.9 L14.4,9.2 Z M18,13 A5,5 0 1 1 17.9,13"); private static readonly Geometry IconFilter = Glyph("M7,7 L31,7 L22,18 L22,29 L16,32 L16,18 Z"); @@ -219,7 +233,7 @@ private static ControlTemplate CreateButtonTemplate() var border = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); border.Name = "ButtonChrome"; border.SetValue(System.Windows.Controls.Border.SnapsToDevicePixelsProperty, true); - border.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(2)); + border.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); border.SetBinding(System.Windows.Controls.Border.BackgroundProperty, new Binding("Background") { RelativeSource = RelativeSource.TemplatedParent }); border.SetBinding(System.Windows.Controls.Border.BorderBrushProperty, new Binding("BorderBrush") { RelativeSource = RelativeSource.TemplatedParent }); border.SetBinding(System.Windows.Controls.Border.BorderThicknessProperty, new Binding("BorderThickness") { RelativeSource = RelativeSource.TemplatedParent }); @@ -238,7 +252,7 @@ private static ControlTemplate CreateButtonTemplate() // hover/press feedback regardless of its base color. var overlay = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); overlay.Name = "HoverOverlay"; - overlay.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(2)); + overlay.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); overlay.SetValue(System.Windows.Controls.Border.BackgroundProperty, Brushes.Transparent); overlay.SetValue(UIElement.IsHitTestVisibleProperty, false); layers.AppendChild(overlay); @@ -247,7 +261,7 @@ private static ControlTemplate CreateButtonTemplate() // button takes keyboard focus, so it is visible on any background. var focusRing = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); focusRing.Name = "FocusRing"; - focusRing.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(2)); + focusRing.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); focusRing.SetValue(System.Windows.Controls.Border.BorderBrushProperty, Brushes.Transparent); focusRing.SetValue(System.Windows.Controls.Border.BorderThicknessProperty, new Thickness(2)); focusRing.SetValue(UIElement.IsHitTestVisibleProperty, false); @@ -272,6 +286,40 @@ private static ControlTemplate CreateButtonTemplate() return template; } + // A focus-visual adorner (drawn over the focused control on keyboard focus): + // a 2px BlueLight rounded border so text box / combo / list focus is visible. + private static Style CreateFocusVisual() + { + var template = new ControlTemplate(); + var border = new FrameworkElementFactory(typeof(System.Windows.Controls.Border)); + border.SetValue(System.Windows.Controls.Border.BorderBrushProperty, BlueLight); + border.SetValue(System.Windows.Controls.Border.BorderThicknessProperty, new Thickness(2)); + border.SetValue(System.Windows.Controls.Border.CornerRadiusProperty, new CornerRadius(4)); + border.SetValue(System.Windows.Controls.Border.SnapsToDevicePixelsProperty, true); + border.SetValue(FrameworkElement.MarginProperty, new Thickness(-2)); + template.VisualTree = border; + var style = new Style(); + style.Setters.Add(new Setter(Control.TemplateProperty, template)); + return style; + } + + // Maps a result's status word to its legend color so the review list is + // colour-coded (eligible = red, kept = muted, deleted = green, protected = + // blue, failed = amber). The colour is paired with the status word, never the + // only signal, so meaning never depends on colour alone. + private static Brush StatusToBrush(string statusLabel) + { + if (string.IsNullOrEmpty(statusLabel)) return Muted; + switch (statusLabel) + { + case "Eligible": return RedText; + case "Deleted": return Green; + case "Protected": return BlueLight; + case "Warning": return Amber; + default: return Muted; // Kept / Ignored / NeverEmpty / etc. + } + } + private void ApplyInitialWindowBounds() { double availableWidth = Math.Max(640d, SystemParameters.WorkArea.Width - 40d); @@ -575,6 +623,7 @@ private UIElement BuildSearchTab() VerticalAlignment = VerticalAlignment.Top, HorizontalAlignment = HorizontalAlignment.Stretch }; + pathBox.FocusVisualStyle = FocusVisual; SetAutomation(pathBox, "Folder to scan", "Enter or paste the root folder RED++ should scan."); pathRow.Children.Add(pathBox); @@ -689,16 +738,18 @@ private Grid BuildEmptyState() VerticalAlignment = VerticalAlignment.Center }; grid.Children.Add(center); - center.Children.Add(IconPath(IconFolder, Muted2, 46, 2.5, new DoubleCollection(new[] { 5d, 4d }), new Thickness(0, 0, 0, 8))); - center.Children.Add(new TextBlock + emptyIcon = IconPath(IconFolder, Muted2, 46, 2.5, new DoubleCollection(new[] { 5d, 4d }), new Thickness(0, 0, 0, 8)); + center.Children.Add(emptyIcon); + emptyTitle = new TextBlock { Text = "Choose a folder to scan.", Foreground = Text, FontSize = 18, TextAlignment = TextAlignment.Center, Margin = new Thickness(0, 0, 0, 8) - }); - center.Children.Add(new TextBlock + }; + center.Children.Add(emptyTitle); + emptySubtitle = new TextBlock { Text = "Review results before anything changes.", Foreground = Muted, @@ -706,13 +757,42 @@ private Grid BuildEmptyState() LineHeight = 18, TextAlignment = TextAlignment.Center, Margin = new Thickness(0, 0, 0, 10) - }); - center.Children.Add(TrustRow(IconSearch, BlueLight, "Pick a root folder, then scan.")); - center.Children.Add(TrustRow(IconShield, Green, "Review eligible results.")); - center.Children.Add(TrustRow(IconTrash, Pink, "Confirm before changes.")); + }; + center.Children.Add(emptySubtitle); + emptyTrust = new StackPanel(); + emptyTrust.Children.Add(TrustRow(IconSearch, BlueLight, "Pick a root folder, then scan.")); + emptyTrust.Children.Add(TrustRow(IconShield, Green, "Review eligible results.")); + emptyTrust.Children.Add(TrustRow(IconTrash, Pink, "Confirm before changes.")); + center.Children.Add(emptyTrust); return grid; } + // Swaps the centre panel between the pre-scan prompt and a positive "all clean" + // state, so a completed scan with no results no longer tells the user to pick a + // folder they just scanned. + private void SetEmptyStateMode(bool clean) + { + if (emptyIcon == null) return; + if (clean) + { + emptyIcon.Data = IconCheck; + emptyIcon.Stroke = Green; + emptyIcon.StrokeDashArray = null; + emptyTitle.Text = "No empty directories found."; + emptySubtitle.Text = "RED++ scanned this folder and found nothing to remove."; + emptyTrust.Visibility = Visibility.Collapsed; + } + else + { + emptyIcon.Data = IconFolder; + emptyIcon.Stroke = Muted2; + emptyIcon.StrokeDashArray = new DoubleCollection(new[] { 5d, 4d }); + emptyTitle.Text = "Choose a folder to scan."; + emptySubtitle.Text = "Review results before anything changes."; + emptyTrust.Visibility = Visibility.Visible; + } + } + private UIElement TrustRow(Geometry icon, Brush brush, string text) { var row = new StackPanel @@ -739,9 +819,30 @@ private ListView BuildResultsList() ScrollViewer.SetHorizontalScrollBarVisibility(list, ScrollBarVisibility.Auto); ScrollViewer.SetVerticalScrollBarVisibility(list, ScrollBarVisibility.Auto); SetAutomation(list, "Review results", "Empty directories and empty files found during the last scan."); + list.FocusVisualStyle = FocusVisual; + + // Comfortable, tappable rows for a destructive-review list. + var itemStyle = new Style(typeof(System.Windows.Controls.ListViewItem)); + itemStyle.Setters.Add(new Setter(Control.MinHeightProperty, 28d)); + itemStyle.Setters.Add(new Setter(Control.PaddingProperty, new Thickness(4, 2, 4, 2))); + itemStyle.Setters.Add(new Setter(Control.VerticalContentAlignmentProperty, VerticalAlignment.Center)); + list.ItemContainerStyle = itemStyle; + var gridView = new GridView(); list.View = gridView; - gridView.Columns.Add(new GridViewColumn { Header = "Status", Width = 96, DisplayMemberBinding = new Binding("StatusLabel") }); + + // Status as a coloured word (eligible = red, kept = muted, deleted = + // green, …) so the review list reflects the legend instead of leaving the + // computed status colour unused. The word carries the meaning; the colour + // reinforces it (never colour alone). + var statusTemplate = new DataTemplate(); + var statusText = new FrameworkElementFactory(typeof(TextBlock)); + statusText.SetBinding(TextBlock.TextProperty, new Binding("StatusLabel")); + statusText.SetBinding(TextBlock.ForegroundProperty, new Binding("StatusBrush")); + statusText.SetValue(TextBlock.FontWeightProperty, FontWeights.SemiBold); + statusTemplate.VisualTree = statusText; + + gridView.Columns.Add(new GridViewColumn { Header = "Status", Width = 96, CellTemplate = statusTemplate }); gridView.Columns.Add(new GridViewColumn { Header = "Name", Width = 170, DisplayMemberBinding = new Binding("Name") }); gridView.Columns.Add(new GridViewColumn { Header = "Reason", Width = 220, DisplayMemberBinding = new Binding("Reason") }); gridView.Columns.Add(new GridViewColumn { Header = "Path", Width = 460, DisplayMemberBinding = new Binding("FullPath") }); @@ -790,6 +891,7 @@ private UIElement BuildSettingsTab() BorderBrush = BorderStrong, BorderThickness = new Thickness(1) }; + deleteMode.FocusVisualStyle = FocusVisual; SetAutomation(deleteMode, "Deletion mode", "Choose whether RED++ simulates, recycles, deletes directly, or moves eligible results."); foreach (DeleteModes mode in DeleteModeItem.GetList()) { @@ -859,6 +961,7 @@ private void AddFilterList(Grid grid, int column, string title, List rul MinHeight = 420, FontSize = 15 }; + list.FocusVisualStyle = FocusVisual; panel.Children.Add(list); } @@ -905,7 +1008,7 @@ private void BuildCommandBar() deleteButton.Click += (s, e) => StartDelete(); primaryActions.Children.Add(deleteButton); - cancelButton = ActionButton("Cancel", BrushFrom("#27334a"), IconCancel, 140); + cancelButton = ActionButton("Cancel", Panel2, IconCancel, 140); SetAutomation(cancelButton, "Cancel current operation", "Cancel the scan or deletion currently in progress."); cancelButton.Margin = new Thickness(12, 0, 0, 0); cancelButton.Click += (s, e) => core?.CancelCurrentProcess(); @@ -935,7 +1038,7 @@ private WpfButton ActionButton(string text, Brush background, Geometry icon, dou Width = width, Height = 52, Background = background, - BorderBrush = BrushFrom("#6f86ad"), + BorderBrush = BorderStrong, BorderThickness = new Thickness(1), Cursor = Cursors.Hand }; @@ -1189,9 +1292,10 @@ private void AttachCoreEvents(REDCore activeCore) AddEmptyFileResults(); int total = e.EmptyFolderCount + e.EmptyFileCount; detailStatusText.Text = total == 0 - ? string.Format("Checked {0} directories. Nothing to delete yet.", e.FolderCount) + ? string.Format("Checked {0} {1} — no empty directories found.", e.FolderCount, e.FolderCount == 1 ? "directory" : "directories") : string.Format("{0} empty directories and {1} empty files eligible.", e.EmptyFolderCount, e.EmptyFileCount); itemCountText.Text = total + " items"; + hasScanned = true; UpdateUiState(false); deleteButton.IsEnabled = total > 0; progressBar.IsIndeterminate = false; @@ -1207,12 +1311,12 @@ private void AttachCoreEvents(REDCore activeCore) activeCore.OnCancelled += (s, e) => Dispatcher.Invoke(() => { UpdateUiState(false); - detailStatusText.Text = "Operation canceled."; + detailStatusText.Text = "Canceled."; }); activeCore.OnAborted += (s, e) => Dispatcher.Invoke(() => { UpdateUiState(false); - detailStatusText.Text = "Operation stopped."; + detailStatusText.Text = "Stopped after an error."; }); activeCore.OnDeleteProcessChanged += (s, e) => Dispatcher.Invoke(() => { @@ -1220,7 +1324,9 @@ private void AttachCoreEvents(REDCore activeCore) AddOrUpdateResult(e.ScanResult); if (rowsByPath.ContainsKey(e.ScanResult.FullPath)) { - rowsByPath[e.ScanResult.FullPath].StatusLabel = e.Status.ToString(); + ResultRow updatedRow = rowsByPath[e.ScanResult.FullPath]; + updatedRow.StatusLabel = e.Status.ToString(); + updatedRow.StatusBrush = StatusToBrush(updatedRow.StatusLabel); } progressBar.IsIndeterminate = false; progressBar.Maximum = Math.Max(1, e.FolderCount); @@ -1247,7 +1353,7 @@ private void AttachCoreEvents(REDCore activeCore) { UpdateUiState(false); deleteButton.IsEnabled = false; - detailStatusText.Text = string.Format("Deletion complete: {0} directories and {1} files changed.", e.DeletedFolderCount, e.DeletedFileCount); + detailStatusText.Text = BuildCompletionMessage(e.DeletedFolderCount, e.DeletedFileCount); progressBar.IsIndeterminate = false; progressBar.Value = 0; progressText.Text = "0%"; @@ -1272,7 +1378,7 @@ private void AddOrUpdateResult(RedScanResultItem item) row.FullPath = item.FullPath; row.Reason = item.StatusReason; row.StatusLabel = item.SearchStatus == DirectorySearchStatusTypes.Empty ? "Eligible" : "Kept"; - row.StatusBrush = item.SearchStatus == DirectorySearchStatusTypes.Empty ? Red : Muted; + row.StatusBrush = StatusToBrush(row.StatusLabel); itemCountText.Text = results.Count + " items"; RefreshResultsVisibility(); } @@ -1387,13 +1493,35 @@ private string BuildDeleteConfirmationMessage(int deleteCount, int fileDeleteCou } } + // Completion copy that matches the chosen mode. Critically, a dry run reports + // "would be removed / nothing was changed" instead of claiming files changed. + private string BuildCompletionMessage(int dirs, int files) + { + string d = dirs == 1 ? "directory" : "directories"; + string f = files == 1 ? "file" : "files"; + DeleteModes mode = runData != null ? runData.DeleteMode : DeleteModes.RecycleBin; + switch (mode) + { + case DeleteModes.Simulate: + return string.Format("Dry run complete — {0} {1} and {2} {3} would be removed. Nothing was changed.", dirs, d, files, f); + case DeleteModes.MoveToFolder: + return string.Format("Moved {0} {1} and {2} {3}.", dirs, d, files, f); + case DeleteModes.Direct: + return string.Format("Deleted {0} {1} and {2} {3}.", dirs, d, files, f); + default: + return string.Format("Recycled {0} {1} and {2} {3}.", dirs, d, files, f); + } + } + private void UpdateUiState(bool busy) { scanButton.IsEnabled = !busy; cancelButton.IsEnabled = busy; extrasButton.IsEnabled = !busy; - readyText.Text = busy ? "● Busy" : "● Ready"; - readyText.Foreground = busy ? BrushFrom("#e8c35b") : Text; + readyText.Text = busy ? "● Working…" : "● Ready"; + readyText.Foreground = busy ? Amber : Green; + // Screen readers should hear the state word, not the decorative bullet. + System.Windows.Automation.AutomationProperties.SetName(readyText, busy ? "Working" : "Ready"); if (!busy && deleteButton != null) { // Enable delete only when something is actually eligible — rows that @@ -1424,6 +1552,7 @@ private void RefreshResultsVisibility() bool empty = results.Count == 0; emptyState.Visibility = empty ? Visibility.Visible : Visibility.Collapsed; resultsList.Visibility = empty ? Visibility.Collapsed : Visibility.Visible; + if (empty) SetEmptyStateMode(hasScanned); } private void ApplyConfigToUi() From 21ca0881ddcffdbc81dbf6eb8413d0eed4e39e3c Mon Sep 17 00:00:00 2001 From: SysAdminDoc Date: Sun, 14 Jun 2026 03:15:47 -0400 Subject: [PATCH 5/5] fix(ui): guard tree protect/unprotect against non-DirectoryInfo nodes unprotectNode/ProtectNode cast node.Tag to DirectoryInfo unconditionally, so a node with a null or placeholder Tag threw while walking the tree. Both now use a pattern-matched guard and skip such nodes (the class's own TODO flagged this). --- RED/RedLib/TreeManager.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/RED/RedLib/TreeManager.cs b/RED/RedLib/TreeManager.cs index c51203d..6f27896 100644 --- a/RED/RedLib/TreeManager.cs +++ b/RED/RedLib/TreeManager.cs @@ -555,10 +555,10 @@ internal void UnprotectSelected() private void unprotectNode(TreeNode node) { - if (node != null) + // A node whose Tag is missing or not a DirectoryInfo (e.g. a placeholder + // child) must be skipped rather than throwing while walking the tree. + if (node?.Tag is DirectoryInfo directory) { - DirectoryInfo directory = ((DirectoryInfo)node.Tag); - if (!this.nodePropsBackup.ContainsKey(directory.FullName)) { // TODO: What to do when this info is missing, show error? @@ -589,10 +589,8 @@ private void unprotectNode(TreeNode node) private void ProtectNode(TreeNode node) { - if (node != null) + if (node?.Tag is DirectoryInfo directory) { - DirectoryInfo directory = (DirectoryInfo)node.Tag; - if (nodePropsBackup.ContainsKey(directory.FullName)) { return;