Deep audit: data-safety, CLI, filter, and WPF UX hardening

CHANGELOG.md

@@ -13,20 +13,33 @@
 - 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`.
 
 ### 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 <name>` stores the current options (paths, mode, empty-files, age/depth, gitignore/MFT/hidden/system toggles) as a named profile; `-profile <name>` 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.)
 
 ### 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.
+- Restore a default sub-object after loading a config whose child element was explicitly nil'd (e.g. a hand-edited `<Options xsi:nil="true"/>`), 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

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<string> Set(params string[] paths)
+        {
+            return new HashSet<string>(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<string>()));
+            Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected(null, Set(@"C:\a")));
+            Assert.False(DeletionWorker.IsProtectedOrAncestorOfProtected("", Set(@"C:\a")));
+        }
+    }
+}

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
+        }
     }
 }

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()
         {

RED/Config/Config.cs

@@ -57,6 +57,22 @@ internal void SetToDefaults()
             UI.SetToDefaults();
         }
 
+        /// <summary>
+        /// Replaces any null sub-object with a fresh default. A hand-crafted config that
+        /// nils a child (e.g. <c>&lt;Options xsi:nil="true"/&gt;</c>) 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.
+        /// </summary>
+        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);

RED/Config/ConfigAssist.cs

@@ -39,6 +39,10 @@ internal static void ConfigLoad(ref RedConfiguration config, string appName)
                     {
                         config = ConfigAssist.Load<RedConfiguration>(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))
                         {

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 <dir>");
+						Environment.ExitCode = 1;
+						return;
+					}
+				}
+
 				var prof = new RedProfile
 				{
 					Name = saveProfileName,

RED/RedLib/Core.cs

@@ -48,8 +48,9 @@ public void SearchingForEmptyDirectories()
 		{
 			CurrentProcessStep = WorkflowSteps.StartSearchingForEmptyDirs;
 
-			// Rest folder list
-			RunData.ProtectedFolderList = new Dictionary<string, bool>();
+			// 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<string, bool>(StringComparer.OrdinalIgnoreCase);
 
 			// Start async empty directory search worker
 			SearchEmptyFoldersWorker = new FindEmptyDirectoryWorker();