fix: survive an unreadable directory while scanning the dev directory [patch]

ProjectDirector.Test/DevDirectoryScanTests.cs

@@ -0,0 +1,207 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.ProjectDirector.Test;
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+/// <summary>
+/// Tests the walk that finds working trees under the configured dev directory.
+/// </summary>
+/// <remarks>
+/// The scan used to hand the whole tree to the recursive form of
+/// <see cref="Directory.EnumerateDirectories(string, string, SearchOption)"/>, which leaves
+/// <see cref="EnumerationOptions.IgnoreInaccessible"/> off and enumerates lazily. One unreadable
+/// folder therefore threw part way through, on the render thread, and took the whole application
+/// with it -- and a dev directory holds exactly the package caches, build output and IDE metadata
+/// that produce such a folder. <see cref="ProjectDirector.EnumerateGitDirectories"/> exists
+/// separately so that rule can be driven without a live ImGui context, the way
+/// <see cref="PullDecisionTests"/> drives the pull rule.
+/// </remarks>
+[TestClass]
+public sealed class DevDirectoryScanTests
+{
+	private static string CreateTree()
+	{
+		string root = Path.Join(Path.GetTempPath(), $"ktsu_pd_scan_{Guid.NewGuid():N}");
+		_ = Directory.CreateDirectory(Path.Join(root, "alpha", ".git"));
+		_ = Directory.CreateDirectory(Path.Join(root, "nested", "beta", ".git"));
+		_ = Directory.CreateDirectory(Path.Join(root, "nested", "beta", "src"));
+		_ = Directory.CreateDirectory(Path.Join(root, "notarepo"));
+		return root;
+	}
+
+	private static string[] Walk(string root, Func<string, string[]>? listDirectories = null) =>
+		[.. ProjectDirector.EnumerateGitDirectories(root, listDirectories)
+			.Select(Path.GetFullPath)
+			.Order(StringComparer.Ordinal)];
+
+	[TestMethod]
+	public void EveryWorkingTreeUnderTheRootIsFound()
+	{
+		string root = CreateTree();
+		try
+		{
+			string[] found = Walk(root);
+
+			CollectionAssert.AreEqual(
+				new[]
+				{
+					Path.GetFullPath(Path.Join(root, "alpha", ".git")),
+					Path.GetFullPath(Path.Join(root, "nested", "beta", ".git")),
+				}.Order(StringComparer.Ordinal).ToArray(),
+				found,
+				"The walk should report every .git directory at any depth and nothing else.");
+		}
+		finally
+		{
+			Directory.Delete(root, recursive: true);
+		}
+	}
+
+	/// <summary>
+	/// The regression this file exists for: a directory that refuses to be listed must cost only
+	/// itself, not the repositories that sort after it.
+	/// </summary>
+	/// <remarks>
+	/// The refusal is injected rather than arranged with file permissions because a test process
+	/// running as root -- which CI containers routinely do -- bypasses the permission bits
+	/// entirely, so <c>chmod</c> would quietly produce a readable directory and the test would pass
+	/// against the unfixed code. <see cref="ADeniedDirectoryOnDiskIsSkipped"/> covers the real
+	/// file system wherever the host can actually deny a read.
+	/// </remarks>
+	[TestMethod]
+	public void ADeniedDirectoryCostsOnlyItsOwnSubtree()
+	{
+		string root = CreateTree();
+		try
+		{
+			string denied = Path.Join(root, "nested");
+			List<string> refused = [];
+
+			string[] found = Walk(root, directory =>
+			{
+				if (string.Equals(directory, denied, StringComparison.Ordinal))
+				{
+					refused.Add(directory);
+					throw new UnauthorizedAccessException($"Access to the path '{directory}' is denied.");
+				}
+
+				return Directory.GetDirectories(directory);
+			});
+
+			Assert.AreEqual(1, refused.Count, "The walk should have reached the denied directory exactly once.");
+			CollectionAssert.AreEqual(
+				new[] { Path.GetFullPath(Path.Join(root, "alpha", ".git")) },
+				found,
+				"The readable half of the tree should survive a directory that refuses to be listed.");
+		}
+		finally
+		{
+			Directory.Delete(root, recursive: true);
+		}
+	}
+
+	[TestMethod]
+	public void ADirectoryThatVanishesMidWalkIsSkipped()
+	{
+		string root = CreateTree();
+		try
+		{
+			string vanished = Path.Join(root, "nested");
+
+			string[] found = Walk(root, directory => string.Equals(directory, vanished, StringComparison.Ordinal)
+				? throw new DirectoryNotFoundException($"Could not find a part of the path '{directory}'.")
+				: Directory.GetDirectories(directory));
+
+			CollectionAssert.AreEqual(
+				new[] { Path.GetFullPath(Path.Join(root, "alpha", ".git")) },
+				found,
+				"A directory removed while the scan is running should not end the scan.");
+		}
+		finally
+		{
+			Directory.Delete(root, recursive: true);
+		}
+	}
+
+	[TestMethod]
+	public void AMissingRootYieldsNothingRatherThanThrowing()
+	{
+		string missing = Path.Join(Path.GetTempPath(), $"ktsu_pd_scan_{Guid.NewGuid():N}");
+
+		CollectionAssert.AreEqual(Array.Empty<string>(), Walk(missing), "A dev directory that does not exist is not a crash.");
+	}
+
+	[TestMethod]
+	public void TheContentsOfAGitDirectoryAreNotWalked()
+	{
+		string root = CreateTree();
+		try
+		{
+			// A working tree checked out inside another repository's storage is not a second
+			// repository, and git's own object store is large enough to be worth not descending into.
+			_ = Directory.CreateDirectory(Path.Join(root, "alpha", ".git", "modules", "sub", ".git"));
+
+			string[] found = Walk(root);
+
+			CollectionAssert.AreEqual(
+				new[]
+				{
+					Path.GetFullPath(Path.Join(root, "alpha", ".git")),
+					Path.GetFullPath(Path.Join(root, "nested", "beta", ".git")),
+				}.Order(StringComparer.Ordinal).ToArray(),
+				found,
+				"The walk should stop at a .git directory rather than descend into it.");
+		}
+		finally
+		{
+			Directory.Delete(root, recursive: true);
+		}
+	}
+
+	/// <summary>
+	/// The same regression against the real file system, for hosts where a read can actually be
+	/// denied. Goes inconclusive rather than passing vacuously where it cannot be.
+	/// </summary>
+	[TestMethod]
+	public void ADeniedDirectoryOnDiskIsSkipped()
+	{
+		if (OperatingSystem.IsWindows())
+		{
+			Assert.Inconclusive("Denying a directory read on Windows needs an ACL edit rather than a mode change.");
+			return;
+		}
+
+		string root = CreateTree();
+		string denied = Path.Join(root, "nested");
+		try
+		{
+			File.SetUnixFileMode(denied, UnixFileMode.None);
+
+			try
+			{
+				_ = Directory.GetDirectories(denied);
+				Assert.Inconclusive("This process reads a mode-000 directory anyway, most likely because it is root.");
+			}
+			catch (UnauthorizedAccessException)
+			{
+				// The mode took effect, so the walk is about to meet a genuinely unreadable directory.
+			}
+
+			CollectionAssert.AreEqual(
+				new[] { Path.GetFullPath(Path.Join(root, "alpha", ".git")) },
+				Walk(root),
+				"The readable half of the tree should survive an unreadable directory on disk.");
+		}
+		finally
+		{
+			File.SetUnixFileMode(denied, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute);
+			Directory.Delete(root, recursive: true);
+		}
+	}
+}

ProjectDirector/ProjectDirector.cs

@@ -18,7 +18,7 @@
 using ktsu.ImGui.Widgets;
 using ktsu.ImGui.Styler;
 using Octokit;
 // using OpenAI.Chat;
 using Semantics.Paths;
 
 #pragma warning disable CA1506
@@ -43,7 +43,7 @@
 	private Collection<RelativePath> BrowserContentsCompare { get; set; } = [];
 	private PopupPropagateFile PopupPropagateFile { get; } = new();
 
 	// private ChatClient ChatClient { get; init; }
 
 	private static void Main(string[] _)
 	{
@@ -66,7 +66,7 @@
 		_ = MakeLoadedOptionsSafe(Options, QueueLog);
 
 		Options.Save();
 		// ChatClient = new(model: "gpt-4o", new ApiKeyCredential(Options.OpenAIToken));
 		DividerDiff = new("DiffDivider", DividerResized, ImGuiWidgets.DividerLayout.Columns);
 		DividerContainerCols = new("VerticalDivider", DividerResized, ImGuiWidgets.DividerLayout.Columns);
 		DividerContainerRows = new("HorizontalDivider", DividerResized, ImGuiWidgets.DividerLayout.Rows);
@@ -604,7 +604,7 @@
 						});
 					}
 
 					//int fetchInterval = repo.MinFetchIntervalSeconds;
 					//if (ImGuiWidgets.Knob("Min Fetch Interval", ref fetchInterval, 0, 300, 150))
 					//{
 					//	repo.MinFetchIntervalSeconds = fetchInterval;
@@ -926,11 +926,75 @@
 	private static FullyQualifiedGitHubRepoName GetFullyQualifiedRepoName(Repository repo) => FullyQualifiedGitHubRepoName.Create<FullyQualifiedGitHubRepoName>(repo.FullName.Replace('/', '.'));
 	private static FullyQualifiedGitHubRepoName GetFullyQualifiedRepoName(GitHubOwnerName ownerName, GitHubRepoName repoName) => FullyQualifiedGitHubRepoName.Create<FullyQualifiedGitHubRepoName>($"{ownerName}.{repoName}");
 
+	/// <summary>
+	/// Walks <paramref name="root"/> for <c>.git</c> directories, skipping any directory that cannot
+	/// be read rather than abandoning the rest of the tree.
+	/// </summary>
+	/// <param name="root">The directory to walk.</param>
+	/// <param name="listDirectories">
+	/// Lists the immediate subdirectories of one directory. Defaults to <see cref="Directory.GetDirectories(string)"/>;
+	/// the tests substitute a lister that denies a chosen directory, which is the one thing a test cannot
+	/// arrange through the file system itself when it runs as a user that bypasses permission checks.
+	/// </param>
+	/// <remarks>
+	/// The recursive form of <see cref="Directory.EnumerateDirectories(string, string, SearchOption)"/>
+	/// leaves <see cref="EnumerationOptions.IgnoreInaccessible"/> off, and it is lazy, so one
+	/// permission-denied folder anywhere under the dev directory throws part way through the walk and
+	/// takes every repository that would have been found after it along with it. A dev directory
+	/// realistically holds package caches, build output and IDE metadata, so such a folder is ordinary
+	/// rather than exotic. Listing a level at a time keeps a refusal local to the directory that
+	/// raised it.
+	/// Descending stops at a <c>.git</c> directory, whose contents are git's own storage and hold no
+	/// further working trees.
+	/// </remarks>
+	internal static IEnumerable<string> EnumerateGitDirectories(string root, Func<string, string[]>? listDirectories = null)
+	{
+		listDirectories ??= Directory.GetDirectories;
+
+		Stack<string> pending = new();
+		pending.Push(root);
+
+		while (pending.Count > 0)
+		{
+			string current = pending.Pop();
+			string[] subdirectories;
+
+			try
+			{
+				subdirectories = listDirectories(current);
+			}
+			catch (UnauthorizedAccessException)
+			{
+				// The process cannot read this directory; the rest of the tree is still worth walking.
+				continue;
+			}
+			catch (IOException)
+			{
+				// Covers a directory removed mid-walk, a dead junction, and an unreadable volume.
+				continue;
+			}
+
+			foreach (string subdirectory in subdirectories)
+			{
+				// Matched without regard to case because that is what the previous pattern match did
+				// on Windows, which is where this application primarily runs.
+				if (string.Equals(Path.GetFileName(subdirectory), ".git", StringComparison.OrdinalIgnoreCase))
+				{
+					yield return subdirectory;
+				}
+				else
+				{
+					pending.Push(subdirectory);
+				}
+			}
+		}
+	}
+
 	[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "<Pending>")]
 	private void ScanDevDirectoryForOwnersAndRepos()
 	{
 		// scan the dev directory for git repos and when we find one we add the owner to the list of owners and the repo to the list of repos
-		IEnumerable<string> gitDirs = Directory.EnumerateDirectories(Options.DevDirectory, ".git", SearchOption.AllDirectories);
+		IEnumerable<string> gitDirs = EnumerateGitDirectories(Options.DevDirectory);
 		foreach (string gitDir in gitDirs)
 		{
 			// The working tree is the parent of the .git directory, so there is nothing to ask git
@@ -1766,7 +1830,7 @@
 				if (ImGui.TableNextColumn())
 				{
 					//if (ImGui.Button($"Propagate Directory###Propagate{path.Replace(Path.DirectorySeparatorChar, '.').Replace(Path.AltDirectorySeparatorChar, '.')}"))
 					//{
 					//	shouldOpenPopup |= true;
 					//	Options.PropagatePath = path;
 					//}