diff --git a/src/ApiService/ApiService/OneFuzzTypes/Enums.cs b/src/ApiService/ApiService/OneFuzzTypes/Enums.cs index b1f7225b5f..4739987e6b 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Enums.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Enums.cs @@ -49,6 +49,7 @@ public enum ErrorCode { ADO_VALIDATION_MISSING_PAT_SCOPES = 492, ADO_WORKITEM_PROCESSING_DISABLED = 494, ADO_VALIDATION_INVALID_PATH = 495, + ADO_VALIDATION_INVALID_PROJECT = 496, // NB: if you update this enum, also update enums.py } diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs index 3780bc1b2b..b1442851ba 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs @@ -89,30 +89,97 @@ private static bool IsTransient(Exception e) { return errorCodes.Any(errorStr.Contains); } - private static async Async.Task ValidatePath(string project, string path, TreeStructureGroup structureGroup, WorkItemTrackingHttpClient client) { - var pathType = (structureGroup == TreeStructureGroup.Areas) ? "Area" : "Iteration"; - var pathParts = path.Split('\\'); - if (!string.Equals(pathParts[0], project, StringComparison.OrdinalIgnoreCase)) { + public static OneFuzzResultVoid ValidateTreePath(IEnumerable path, WorkItemClassificationNode? root) { + if (root is null) { + return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PROJECT, new string[] { + $"Path \"{string.Join('\\', path)}\" is invalid. The specified ADO project doesn't exist.", + "Double check the 'project' field in your ADO config.", + }); + } + + string treeNodeTypeName; + switch (root.StructureType) { + case TreeNodeStructureType.Area: + treeNodeTypeName = "Area"; + break; + case TreeNodeStructureType.Iteration: + treeNodeTypeName = "Iteration"; + break; + default: + return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { + $"Path root \"{root.Name}\" is an unsupported type. Expected Area or Iteration but got {root.StructureType}.", + }); + } + + // Validate path based on + // https://learn.microsoft.com/en-us/azure/devops/organizations/settings/about-areas-iterations?view=azure-devops#naming-restrictions + var maxNodeLength = 255; + var maxDepth = 13; + // Invalid characters from the link above plus the escape sequences (since they have backslashes and produce confusingly formatted errors if not caught here) + var invalidChars = new char[] { '/', ':', '*', '?', '"', '<', '>', '|', ';', '#', '$', '*', '{', '}', ',', '+', '=', '[', ']' }; + + // Ensure that none of the path parts are too long + var erroneous = path.FirstOrDefault(part => part.Length > maxNodeLength); + if (erroneous != null) { + return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { + $"{treeNodeTypeName} Path \"{string.Join('\\', path)}\" is invalid. \"{erroneous}\" is too long. It must be less than {maxNodeLength} characters.", + "Learn more about naming restrictions here: https://learn.microsoft.com/en-us/azure/devops/organizations/settings/about-areas-iterations?view=azure-devops#naming-restrictions" + }); + } + + // Ensure that none of the path parts contain invalid characters + erroneous = path.FirstOrDefault(part => invalidChars.Any(part.Contains)); + if (erroneous != null) { return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { - $"Path \"{path}\" is invalid. It must start with the project name, \"{project}\".", - $"Example: \"{project}\\{path}\".", + $"{treeNodeTypeName} Path \"{string.Join('\\', path)}\" is invalid. \"{erroneous}\" contains an invalid character ({string.Join(" ", invalidChars)}).", + "Make sure that the path is separated by backslashes (\\) and not forward slashes (/).", + "Learn more about naming restrictions here: https://learn.microsoft.com/en-us/azure/devops/organizations/settings/about-areas-iterations?view=azure-devops#naming-restrictions" }); } - var current = await client.GetClassificationNodeAsync(project, structureGroup, depth: pathParts.Length - 1); - if (current == null) { + // Ensure no unicode control characters + erroneous = path.FirstOrDefault(part => part.Any(ch => char.IsControl(ch))); + if (erroneous != null) { return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { - $"{pathType} Path \"{path}\" is invalid. \"{project}\" is not a valid project.", + // More about control codes and their range here: https://en.wikipedia.org/wiki/Unicode_control_characters + $"{treeNodeTypeName} Path \"{string.Join('\\', path)}\" is invalid. \"{erroneous}\" contains a unicode control character (\\u0000 - \\u001F or \\u007F - \\u009F).", + "Make sure that you're path doesn't contain any escape characters (\\0 \\a \\b \\f \\n \\r \\t \\v).", + "Learn more about naming restrictions here: https://learn.microsoft.com/en-us/azure/devops/organizations/settings/about-areas-iterations?view=azure-devops#naming-restrictions" }); } - foreach (var part in pathParts.Skip(1)) { + // Ensure that there aren't too many path parts + if (path.Count() > maxDepth) { + return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { + $"{treeNodeTypeName} Path \"{string.Join('\\', path)}\" is invalid. It must be less than {maxDepth} levels deep.", + "Learn more about naming restrictions here: https://learn.microsoft.com/en-us/azure/devops/organizations/settings/about-areas-iterations?view=azure-devops#naming-restrictions" + }); + } + + + // Path should always start with the project name ADO expects an absolute path + if (!string.Equals(path.First(), root.Name, StringComparison.OrdinalIgnoreCase)) { + return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { + $"{treeNodeTypeName} Path \"{string.Join('\\', path)}\" is invalid. It must start with the project name, \"{root.Name}\".", + $"Example: \"{root.Name}\\{path}\".", + }); + } + + // Validate that each part of the path is a valid child of the previous part + var current = root; + foreach (var part in path.Skip(1)) { var child = current.Children?.FirstOrDefault(x => string.Equals(x.Name, part, StringComparison.OrdinalIgnoreCase)); if (child == null) { - return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { - $"{pathType} Path \"{path}\" is invalid. \"{part}\" is not a valid child of \"{current.Name}\".", - $"Valid children of \"{current.Name}\" are: [{string.Join(',', current.Children?.Select(x => $"\"{x.Name}\"") ?? new List())}].", - }); + if (current.Children is null || !current.Children.Any()) { + return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { + $"{treeNodeTypeName} Path \"{string.Join('\\', path)}\" is invalid. \"{current.Name}\" has no children.", + }); + } else { + return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] { + $"{treeNodeTypeName} Path \"{string.Join('\\', path)}\" is invalid. \"{part}\" is not a valid child of \"{current.Name}\".", + $"Valid children of \"{current.Name}\" are: [{string.Join(',', current.Children?.Select(x => $"\"{x.Name}\"") ?? new List())}].", + }); + } } current = child; @@ -195,14 +262,19 @@ await policy.ExecuteAsync(async () => { try { // Validate AreaPath and IterationPath exist + // This also validates that the config.Project exists if (config.AdoFields.TryGetValue("System.AreaPath", out var areaPathString)) { - var validateAreaPath = await ValidatePath(config.Project, areaPathString, TreeStructureGroup.Areas, witClient); + var path = areaPathString.Split('\\'); + var root = await witClient.GetClassificationNodeAsync(config.Project, TreeStructureGroup.Areas, depth: path.Length - 1); + var validateAreaPath = ValidateTreePath(path, root); if (!validateAreaPath.IsOk) { return validateAreaPath; } } if (config.AdoFields.TryGetValue("System.IterationPath", out var iterationPathString)) { - var validateIterationPath = await ValidatePath(config.Project, iterationPathString, TreeStructureGroup.Iterations, witClient); + var path = iterationPathString.Split('\\'); + var root = await witClient.GetClassificationNodeAsync(config.Project, TreeStructureGroup.Iterations, depth: path.Length - 1); + var validateIterationPath = ValidateTreePath(path, root); if (!validateIterationPath.IsOk) { return validateIterationPath; } diff --git a/src/ApiService/Tests/TreePathTests.cs b/src/ApiService/Tests/TreePathTests.cs new file mode 100644 index 0000000000..fba818793c --- /dev/null +++ b/src/ApiService/Tests/TreePathTests.cs @@ -0,0 +1,148 @@ +using System.Collections.Generic; +using System.Linq; +using Microsoft.OneFuzz.Service; +using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; +using Xunit; + +namespace Tests; + +// This might be a good candidate for property based testing +// https://fscheck.github.io/FsCheck//QuickStart.html +public class TreePathTests { + private static IEnumerable SplitPath(string path) { + return path.Split('\\'); + } + + private static WorkItemClassificationNode MockTreeNode(IEnumerable path, TreeNodeStructureType structureType) { + var root = new WorkItemClassificationNode() { + Name = path.First(), + StructureType = structureType + }; + + var current = root; + foreach (var segment in path.Skip(1)) { + var child = new WorkItemClassificationNode { + Name = segment + }; + current.Children = new[] { child }; + current = child; + } + + return root; + } + + + [Fact] + public void TestValidPath() { + var path = SplitPath(@"project\foo\bar\baz"); + var root = MockTreeNode(path, TreeNodeStructureType.Area); + + var result = Ado.ValidateTreePath(path, root); + + Assert.True(result.IsOk); + } + + [Fact] + public void TestNullTreeNode() { // A null tree node indicates an invalid ADO project was used in the query + var path = SplitPath(@"project\foo\bar\baz"); + + var result = Ado.ValidateTreePath(path, null); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PROJECT, result.ErrorV!.Code); + Assert.Contains("ADO project doesn't exist", result.ErrorV!.Errors![0]); + } + + [Fact] + public void TestPathPartTooLong() { + var path = SplitPath(@"project\foo\barbazquxquuxcorgegraultgarplywaldofredplughxyzzythudbarbazquxquuxcorgegraultgarplywaldofredplughxyzzythudbarbazquxquuxcorgegraultgarplywaldofredplughxyzzythudbarbazquxquuxcorgegraultgarplywaldofredplughxyzzythudbarbazquxquuxcorgegraultgarplywaldofredplughxyzzythud\baz"); + var root = MockTreeNode(path, TreeNodeStructureType.Iteration); + + var result = Ado.ValidateTreePath(path, root); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PATH, result.ErrorV!.Code); + Assert.Contains("too long", result.ErrorV!.Errors![0]); + } + + [Theory] + [InlineData("project/foo/bar/baz")] + [InlineData("project\\foo:\\bar\\baz")] + public void TestPathContainsInvalidChar(string invalidPath) { + var path = SplitPath(invalidPath); + var treePath = SplitPath(@"project\foo\bar\baz"); + var root = MockTreeNode(treePath, TreeNodeStructureType.Area); + + var result = Ado.ValidateTreePath(path, root); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PATH, result.ErrorV!.Code); + Assert.Contains("invalid character", result.ErrorV!.Errors![0]); + } + + [Theory] + [InlineData("project\\foo\\ba\u0005r\\baz")] + [InlineData("project\\\nfoo\\bar\\baz")] + public void TestPathContainsUnicodeControlChar(string invalidPath) { + var path = SplitPath(invalidPath); + var treePath = SplitPath(@"project\foo\bar\baz"); + var root = MockTreeNode(treePath, TreeNodeStructureType.Area); + + var result = Ado.ValidateTreePath(path, root); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PATH, result.ErrorV!.Code); + Assert.Contains("unicode control character", result.ErrorV!.Errors![0]); + } + + [Fact] + public void TestPathTooDeep() { + var path = SplitPath(@"project\foo\bar\baz\qux\quux\corge\grault\garply\waldo\fred\plugh\xyzzy\thud"); + var root = MockTreeNode(path, TreeNodeStructureType.Area); + + var result = Ado.ValidateTreePath(path, root); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PATH, result.ErrorV!.Code); + Assert.Contains("levels deep", result.ErrorV!.Errors![0]); + } + + [Fact] + public void TestPathWithoutProjectName() { + var path = SplitPath(@"foo\bar\baz"); + var treePath = SplitPath(@"project\foo\bar\baz"); + var root = MockTreeNode(treePath, TreeNodeStructureType.Iteration); + + var result = Ado.ValidateTreePath(path, root); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PATH, result.ErrorV!.Code); + Assert.Contains("start with the project name", result.ErrorV!.Errors![0]); + } + + [Fact] + public void TestPathWithInvalidChild() { + var path = SplitPath(@"project\foo\baz"); + var treePath = SplitPath(@"project\foo\bar"); + var root = MockTreeNode(treePath, TreeNodeStructureType.Iteration); + + var result = Ado.ValidateTreePath(path, root); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PATH, result.ErrorV!.Code); + Assert.Contains("not a valid child", result.ErrorV!.Errors![0]); + } + + [Fact] + public void TestPathWithExtraChild() { + var path = SplitPath(@"project\foo\bar\baz"); + var treePath = SplitPath(@"project\foo\bar"); + var root = MockTreeNode(treePath, TreeNodeStructureType.Iteration); + + var result = Ado.ValidateTreePath(path, root); + + Assert.False(result.IsOk); + Assert.Equal(ErrorCode.ADO_VALIDATION_INVALID_PATH, result.ErrorV!.Code); + Assert.Contains("has no children", result.ErrorV!.Errors![0]); + } +} diff --git a/src/pytypes/onefuzztypes/enums.py b/src/pytypes/onefuzztypes/enums.py index 4d59945c85..e2ec81eb15 100644 --- a/src/pytypes/onefuzztypes/enums.py +++ b/src/pytypes/onefuzztypes/enums.py @@ -303,6 +303,7 @@ class ErrorCode(Enum): ADO_VALIDATION_UNEXPECTED_ERROR = 491 ADO_VALIDATION_MISSING_PAT_SCOPES = 492 ADO_VALIDATION_INVALID_PATH = 495 + ADO_VALIDATION_INVALID_PROJECT = 496 # NB: if you update this enum, also update Enums.cs