Skip to content

Commit

Permalink
Handle unauthorized files / dirs for cleanup (#1328)
Browse files Browse the repository at this point in the history 
* handle unauthorized files / dirs for cleanup

* add comments and unit tests
  • Loading branch information
@pauld-msft
pauld-msft authored Jan 21, 2025
1 parent 91378b6 commit 269fb53
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 56 deletions.
123 changes: 69 additions & 54 deletions src/Microsoft.ComponentDetection.Contracts/FileComponentDetectorWithCleanup.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> detectorArgs, bool cleanupCreatedFiles, CancellationToken cancellationToken = default) =>
Expand All @@ -151,4 +151,19 @@ private bool TryGetCleanupFileDirectory(ProcessRequest processRequest, out strin

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>());
}
}
}
41 changes: 39 additions & 2 deletions test/Microsoft.ComponentDetection.Contracts.Tests/FileComponentDetectorWithCleanupTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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("\\*", ".*") + "$");
Expand Down

0 comments on commit 269fb53

Please sign in to comment.