diff --git a/src/Microsoft.ComponentDetection.Contracts/FileComponentDetectorWithCleanup.cs b/src/Microsoft.ComponentDetection.Contracts/FileComponentDetectorWithCleanup.cs index ddc334a59..0c230e4ca 100644 --- a/src/Microsoft.ComponentDetection.Contracts/FileComponentDetectorWithCleanup.cs +++ b/src/Microsoft.ComponentDetection.Contracts/FileComponentDetectorWithCleanup.cs @@ -56,80 +56,80 @@ protected async Task WithCleanupAsync( // 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 detectorArgs, bool cleanupCreatedFiles, CancellationToken cancellationToken = default) => @@ -151,4 +151,19 @@ private bool TryGetCleanupFileDirectory(ProcessRequest processRequest, out strin return false; } + + private (bool Success, HashSet Files, HashSet Directories) TryGetFilesAndDirectories(string root, IList 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(), new HashSet()); + } + } } diff --git a/test/Microsoft.ComponentDetection.Contracts.Tests/FileComponentDetectorWithCleanupTests.cs b/test/Microsoft.ComponentDetection.Contracts.Tests/FileComponentDetectorWithCleanupTests.cs index 23e8165b2..116b8f62e 100644 --- a/test/Microsoft.ComponentDetection.Contracts.Tests/FileComponentDetectorWithCleanupTests.cs +++ b/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(), It.IsAny>(), It.IsAny()), + 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(); + 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(), It.IsAny>(), It.IsAny())) + .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("\\*", ".*") + "$");