Handle unauthorized files / dirs for cleanup

src/Microsoft.ComponentDetection.Contracts/FileComponentDetectorWithCleanup.cs

@@ -56,80 +56,80 @@
         // determining the files that exist as there will be no subsequent cleanup process.
         if (this.FileUtilityService == null
             || this.DirectoryUtilityService == null
-            || !this.TryGetCleanupFileDirectory(processRequest, out var fileParentDirectory))
+            || !this.TryGetCleanupFileDirectory(processRequest, out var fileParentDirectory)
+            || !cleanupCreatedFiles)
         {
             await process(processRequest, detectorArgs, cancellationToken).ConfigureAwait(false);
             return;
         }
 
         // Get the files and directories that match the cleanup pattern and exist before the process runs.
-        var (preExistingFiles, preExistingDirs) = this.DirectoryUtilityService.GetFilesAndDirectories(fileParentDirectory, this.CleanupPatterns, DefaultCleanDepth);
-        try
+        var (preSuccess, preExistingFiles, preExistingDirs) = this.TryGetFilesAndDirectories(fileParentDirectory, this.CleanupPatterns, DefaultCleanDepth);
+
+        await process(processRequest, detectorArgs, cancellationToken).ConfigureAwait(false);
+        if (!preSuccess)
         {
-            await process(processRequest, detectorArgs, cancellationToken).ConfigureAwait(false);
+            // return early if we failed to get the pre-existing files and directories, since no need for cleanup
+            return;
         }
-        finally
+
+        // Ensure that only one cleanup process is running at a time, helping to prevent conflicts
+        await this.cleanupSemaphore.WaitAsync(cancellationToken).ConfigureAwait(false);
+
+        try
         {
-            // Ensure that only one cleanup process is running at a time, helping to prevent conflicts
-            await this.cleanupSemaphore.WaitAsync(cancellationToken).ConfigureAwait(false);
+            // Clean up any new files or directories created during the scan that match the clean up patterns.
+            var (postSuccess, latestFiles, latestDirs) = this.TryGetFilesAndDirectories(fileParentDirectory, this.CleanupPatterns, DefaultCleanDepth);
+            if (!postSuccess)
+            {
+                // return early if we failed to get the latest files and directories, since we will not be able
+                // to determine what to clean up
+                return;
+            }
 
-            try
+            var createdFiles = latestFiles.Except(preExistingFiles).ToList();
+            var createdDirs = latestDirs.Except(preExistingDirs).ToList();
+
+            foreach (var createdDir in createdDirs)
             {
-                // Clean up any new files or directories created during the scan that match the clean up patterns.
-                // If the cleanupCreatedFiles flag is set to false, this will be a dry run and will just log the files that it would clean.
-                var dryRun = !cleanupCreatedFiles;
-                var dryRunStr = dryRun ? "[DRYRUN] " : string.Empty;
-                var (latestFiles, latestDirs) = this.DirectoryUtilityService.GetFilesAndDirectories(fileParentDirectory, this.CleanupPatterns, DefaultCleanDepth);
-                var createdFiles = latestFiles.Except(preExistingFiles).ToList();
-                var createdDirs = latestDirs.Except(preExistingDirs).ToList();
-
-                foreach (var createdDir in createdDirs)
+                if (createdDir is null || !this.DirectoryUtilityService.Exists(createdDir))
                 {
-                    if (createdDir is null || !this.DirectoryUtilityService.Exists(createdDir))
-                    {
-                        continue;
-                    }
-
-                    try
-                    {
-                        this.Logger.LogDebug("{DryRun}Cleaning up directory {Dir}", dryRunStr, createdDir);
-                        if (!dryRun)
-                        {
-                            this.DirectoryUtilityService.Delete(createdDir, true);
-                        }
-                    }
-                    catch (Exception e)
-                    {
-                        this.Logger.LogDebug(e, "{DryRun}Failed to delete directory {Dir}", dryRunStr, createdDir);
-                    }
+                    continue;
                 }
 
-                foreach (var createdFile in createdFiles)
+                try
+                {
+                    this.Logger.LogDebug("Cleaning up directory {Dir}", createdDir);
+                    this.DirectoryUtilityService.Delete(createdDir, true);
+                }
+                catch (Exception e)
                 {
-                    if (createdFile is null || !this.FileUtilityService.Exists(createdFile))
-                    {
-                        continue;
-                    }
-
-                    try
-                    {
-                        this.Logger.LogDebug("{DryRun}Cleaning up file {File}", dryRunStr, createdFile);
-                        if (!dryRun)
-                        {
-                            this.FileUtilityService.Delete(createdFile);
-                        }
-                    }
-                    catch (Exception e)
-                    {
-                        this.Logger.LogDebug(e, "{DryRun}Failed to delete file {File}", dryRunStr, createdFile);
-                    }
+                    this.Logger.LogDebug(e, "Failed to delete directory {Dir}", createdDir);
                 }
             }
-            finally
+
+            foreach (var createdFile in createdFiles)
             {
-                _ = this.cleanupSemaphore.Release();
+                if (createdFile is null || !this.FileUtilityService.Exists(createdFile))
+                {
+                    continue;
+                }
+
+                try
+                {
+                    this.Logger.LogDebug("Cleaning up file {File}", createdFile);
+                    this.FileUtilityService.Delete(createdFile);
+                }
+                catch (Exception e)
+                {
+                    this.Logger.LogDebug(e, "Failed to delete file {File}", createdFile);
+                }
             }
         }
+        finally
+        {
+            _ = this.cleanupSemaphore.Release();
+        }
     }
 
     protected override async Task OnFileFoundAsync(ProcessRequest processRequest, IDictionary<string, string> detectorArgs, bool cleanupCreatedFiles, CancellationToken cancellationToken = default) =>
@@ -151,4 +151,19 @@
 
         return false;
     }
+
+    private (bool Success, HashSet<string> Files, HashSet<string> Directories) TryGetFilesAndDirectories(string root, IList<string> patterns, int depth)
+    {
+        try
+        {
+            var (files, directories) = this.DirectoryUtilityService.GetFilesAndDirectories(root, patterns, depth);
+            return (true, files, directories);
+        }
+        catch (UnauthorizedAccessException e)
+        {
+            // log and return false if we are unauthorized to get files and directories
+            this.Logger.LogDebug(e, "Unauthorized to get files and directories for {Root}", root);
+            return (false, new HashSet<string>(), new HashSet<string>());
+        }
+    }
 }

test/Microsoft.ComponentDetection.Contracts.Tests/FileComponentDetectorWithCleanupTests.cs

@@ -111,8 +111,6 @@ public async Task WithCleanupAsync_CleansUpCreatedFile()
         var fileComponentDetector = new TestFileComponentDetector(["*.pyc"], this.loggerMock.Object, fileUtilityService, directoryUtilityService);
         var createdFilePath = Path.Combine(this.testDirectory, "todelete.pyc");
 
-        // Add files/dirs to the mock file system
-
         // Act
         await fileComponentDetector.TestCleanupAsync(
             (process, args, token) =>
@@ -198,6 +196,11 @@ await fileComponentDetector.TestCleanupAsync(
         fileUtilityService.Exists(this.existingFilePath).Should().BeTrue();
         directoryUtilityService.Exists(createdDirectory).Should().BeTrue();
         fileUtilityService.Exists(createdFilePath).Should().BeTrue();
+
+        // Assert we don't even try to read items
+        this.directoryUtilityServiceMock.Verify(
+            d => d.GetFilesAndDirectories(It.IsAny<string>(), It.IsAny<List<string>>(), It.IsAny<int>()),
+            Times.Never);
     }
 
     [TestMethod]
@@ -292,6 +295,40 @@ await fileComponentDetector.TestCleanupAsync(
         fileUtilityService.Exists(createdFilePath).Should().BeTrue();
     }
 
+    [TestMethod]
+    public async Task WithCleanupAsync_NoCleanup_WhenUnauthorized()
+    {
+        // Arrange
+        var fileUtilityService = this.fileUtilityServiceMock.Object;
+        var directoryUtilityService = this.directoryUtilityServiceMock.Object;
+        CancellationToken cancellationToken = default;
+        var detectorArgs = new Dictionary<string, string>();
+        var cleanupCreatedFiles = true;
+        var fileComponentDetector = new TestFileComponentDetector(["*.pyc"], this.loggerMock.Object, fileUtilityService, directoryUtilityService);
+        var createdFilePath = Path.Combine(this.testDirectory, "todelete.pyc");
+        this.directoryUtilityServiceMock
+            .Setup(d => d.GetFilesAndDirectories(It.IsAny<string>(), It.IsAny<List<string>>(), It.IsAny<int>()))
+            .Throws(new UnauthorizedAccessException("Unauthorized"));
+
+        // Act
+        await fileComponentDetector.TestCleanupAsync(
+            (process, args, token) =>
+            {
+                // creates a single file
+                this.fileSystemMockFiles.Add(createdFilePath);
+                return Task.CompletedTask;
+            },
+            this.processRequest,
+            detectorArgs,
+            cleanupCreatedFiles,
+            cancellationToken).ConfigureAwait(false);
+
+        // Assert
+        directoryUtilityService.Exists(this.testDirectory).Should().BeTrue();
+        fileUtilityService.Exists(this.existingFilePath).Should().BeTrue();
+        fileUtilityService.Exists(createdFilePath).Should().BeTrue();
+    }
+
     private bool IsPatternMatch(string path, string pattern)
     {
         return Regex.IsMatch(path, "^" + Regex.Escape(pattern).Replace("\\*", ".*") + "$");