fix(template-folder): bound cycle-walk to defend against malformed graphs

This commit is contained in:
Joseph Doherty
2026-05-11 10:58:02 -04:00
parent 1269054651
commit e44bbc0caf
2 changed files with 24 additions and 1 deletions

View File

@@ -86,13 +86,17 @@ public class TemplateFolderService
return Result<TemplateFolder>.Failure($"Target folder with ID {newParentId.Value} not found."); return Result<TemplateFolder>.Failure($"Target folder with ID {newParentId.Value} not found.");
var all = await _repository.GetAllFoldersAsync(cancellationToken); var all = await _repository.GetAllFoldersAsync(cancellationToken);
// Walk up from newParentId — if we encounter folderId, the move would create a cycle.
var byId = all.ToDictionary(f => f.Id); var byId = all.ToDictionary(f => f.Id);
var cursor = newParentId; var cursor = newParentId;
// Walk up from newParentId — if we encounter folderId, the move would create a cycle.
// Bound iterations by byId.Count to defensively terminate on a malformed graph.
var iterations = 0;
while (cursor.HasValue) while (cursor.HasValue)
{ {
if (cursor.Value == folderId) if (cursor.Value == folderId)
return Result<TemplateFolder>.Failure("Cannot move a folder under one of its descendants (cycle)."); return Result<TemplateFolder>.Failure("Cannot move a folder under one of its descendants (cycle).");
if (++iterations > byId.Count)
return Result<TemplateFolder>.Failure("Folder hierarchy contains a cycle; cannot determine ancestry.");
cursor = byId.TryGetValue(cursor.Value, out var node) ? node.ParentFolderId : null; cursor = byId.TryGetValue(cursor.Value, out var node) ? node.ParentFolderId : null;
} }

View File

@@ -167,4 +167,23 @@ public class TemplateFolderServiceTests
Assert.True(result.IsSuccess); Assert.True(result.IsSuccess);
Assert.Null(result.Value.ParentFolderId); Assert.Null(result.Value.ParentFolderId);
} }
[Fact]
public async Task MoveFolder_PreExistingCycleInGraph_ReturnsFailure_DoesNotInfiniteLoop()
{
// Manufactured malformed graph: X.parent=Y, Y.parent=X. We move Z under X.
// The ancestor walk would loop forever without a guard.
var x = new TemplateFolder("X") { Id = 1, ParentFolderId = 2 };
var y = new TemplateFolder("Y") { Id = 2, ParentFolderId = 1 };
var z = new TemplateFolder("Z") { Id = 3, ParentFolderId = null };
_repoMock.Setup(r => r.GetFolderByIdAsync(3, It.IsAny<CancellationToken>())).ReturnsAsync(z);
_repoMock.Setup(r => r.GetFolderByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(x);
_repoMock.Setup(r => r.GetAllFoldersAsync(It.IsAny<CancellationToken>()))
.ReturnsAsync(new List<TemplateFolder> { x, y, z });
var result = await _sut.MoveFolderAsync(3, 1, "admin");
Assert.True(result.IsFailure);
Assert.Contains("cycle", result.Error, StringComparison.OrdinalIgnoreCase);
}
} }