From 2bcfdf3d4298aa64add18ced281503c70ebb4057 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:47:58 -0700 Subject: [PATCH 1/8] add redirect URL and docfx file path checks --- .../DocfxVerifier/DocfxVerifier.csproj | 9 + .../DocfxVerifier/PathVerifier.cs | 210 ++++++++++++++++++ actions/docs-verifier/MSDocsBuildVerifier.sln | 18 +- .../src/ActionRunner/ActionRunner.csproj | 1 + .../docs-verifier/src/ActionRunner/Program.cs | 46 +++- .../RedirectTargetVerifier.cs | 105 +++++++++ .../GitHub.UnitTests/GitHub.UnitTests.csproj | 4 +- .../GitHub.UnitTests/PathVerifierTests.cs | 136 ++++++++++++ .../RedirectTargetVerifierTests.cs | 157 +++++++++++++ 9 files changed, 678 insertions(+), 8 deletions(-) create mode 100644 actions/docs-verifier/DocfxVerifier/DocfxVerifier.csproj create mode 100644 actions/docs-verifier/DocfxVerifier/PathVerifier.cs create mode 100644 actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs create mode 100644 actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs create mode 100644 actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs diff --git a/actions/docs-verifier/DocfxVerifier/DocfxVerifier.csproj b/actions/docs-verifier/DocfxVerifier/DocfxVerifier.csproj new file mode 100644 index 00000000..b7601447 --- /dev/null +++ b/actions/docs-verifier/DocfxVerifier/DocfxVerifier.csproj @@ -0,0 +1,9 @@ + + + + net10.0 + enable + enable + + + diff --git a/actions/docs-verifier/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/DocfxVerifier/PathVerifier.cs new file mode 100644 index 00000000..0a64e71b --- /dev/null +++ b/actions/docs-verifier/DocfxVerifier/PathVerifier.cs @@ -0,0 +1,210 @@ +using System.Text.Json; + +namespace DocfxVerifier +{ + /// + /// Validates file path entries declared in a docfx.json file. + /// + public static class PathVerifier + { + private static readonly HashSet s_pathStringPropertyNames = + [ + "src", + "dest" + ]; + + private static readonly HashSet s_pathArrayPropertyNames = + [ + "files", + "exclude", + "xref", + "template", + "resource", + "overwrite", + "globalMetadataFiles", + "fileMetadataFiles" + ]; + + private static readonly HashSet s_pathObjectKeyPropertyNames = + [ + "fileMetadata", + "open_to_public_contributors", + "ms.collection", + "ms.custom", + "ms.update-cycle", + "no-loc" + ]; + + /// + /// Verifies that file paths in the docfx.json file are valid. + /// + public static async Task WriteResultsAsync(TextWriter writer) + { + ArgumentNullException.ThrowIfNull(writer, nameof(writer)); + + string? configurationPath = FindDocfxConfigurationPath(); + if (configurationPath is null) + { + await writer.WriteLineAsync("::error::Unable to find docfx.json in the repository root or its immediate subdirectories."); + return false; + } + + using FileStream stream = File.OpenRead(configurationPath); + using JsonDocument json = await JsonDocument.ParseAsync(stream); + + string repositoryRoot = Directory.GetCurrentDirectory(); + string configurationDirectory = Path.GetDirectoryName(Path.GetFullPath(configurationPath)) ?? repositoryRoot; + string configurationPathForLog = configurationPath.Replace('\\', '/'); + + var errors = new List(); + ValidateElement(json.RootElement, null, "$", repositoryRoot, configurationDirectory, errors); + + foreach (string error in errors) + { + await writer.WriteLineAsync($"::error file={configurationPathForLog}::{error}"); + } + + return errors.Count == 0; + } + + private static void ValidateElement( + JsonElement element, + string? propertyName, + string jsonPath, + string repositoryRoot, + string configurationDirectory, + List errors) + { + if (element.ValueKind == JsonValueKind.Object) + { + foreach (JsonProperty property in element.EnumerateObject()) + { + string childPath = $"{jsonPath}.{property.Name}"; + ValidateElement(property.Value, property.Name, childPath, repositoryRoot, configurationDirectory, errors); + } + + if (propertyName is not null && s_pathObjectKeyPropertyNames.Contains(propertyName)) + { + foreach (JsonProperty property in element.EnumerateObject()) + { + ValidatePath(property.Name, $"{jsonPath}.{property.Name}", repositoryRoot, configurationDirectory, errors); + } + } + + return; + } + + if (element.ValueKind == JsonValueKind.Array) + { + if (propertyName is not null && s_pathArrayPropertyNames.Contains(propertyName)) + { + int index = 0; + foreach (JsonElement item in element.EnumerateArray()) + { + if (item.ValueKind == JsonValueKind.String) + { + ValidatePath(item.GetString(), $"{jsonPath}[{index}]", repositoryRoot, configurationDirectory, errors); + } + + index++; + } + } + else + { + int index = 0; + foreach (JsonElement item in element.EnumerateArray()) + { + ValidateElement(item, propertyName, $"{jsonPath}[{index}]", repositoryRoot, configurationDirectory, errors); + index++; + } + } + + return; + } + + if (element.ValueKind == JsonValueKind.String + && propertyName is not null + && s_pathStringPropertyNames.Contains(propertyName)) + { + ValidatePath(element.GetString(), jsonPath, repositoryRoot, configurationDirectory, errors); + } + } + + private static void ValidatePath( + string? path, + string jsonPath, + string repositoryRoot, + string configurationDirectory, + List errors) + { + if (string.IsNullOrWhiteSpace(path) || path is ".") + { + return; + } + + if (Uri.TryCreate(path, UriKind.Absolute, out _)) + { + return; + } + + string normalizedPath = path.Replace('\\', '/'); + string nonWildcardPrefix = GetNonWildcardPrefix(normalizedPath); + if (string.IsNullOrEmpty(nonWildcardPrefix)) + { + return; + } + + bool existsRelativeToRoot = ExistsInRepository(repositoryRoot, nonWildcardPrefix); + bool existsRelativeToConfig = ExistsInRepository(configurationDirectory, nonWildcardPrefix); + if (!existsRelativeToRoot && !existsRelativeToConfig) + { + errors.Add($"{jsonPath}: Path '{path}' is invalid. Checked '{nonWildcardPrefix}' relative to repository root and config directory."); + } + } + + private static bool ExistsInRepository(string baseDirectory, string path) + { + string combinedPath = Path.GetFullPath(Path.Combine(baseDirectory, path)); + return File.Exists(combinedPath) || Directory.Exists(combinedPath); + } + + private static string GetNonWildcardPrefix(string path) + { + ReadOnlySpan wildcardChars = ['*', '?', '[', ']', '{', '}']; + string[] segments = path.Split('/', StringSplitOptions.RemoveEmptyEntries); + + var prefixSegments = new List(); + foreach (string segment in segments) + { + if (segment.AsSpan().IndexOfAny(wildcardChars) >= 0) + { + break; + } + + prefixSegments.Add(segment); + } + + return string.Join('/', prefixSegments); + } + + private static string? FindDocfxConfigurationPath() + { + const string fileName = "docfx.json"; + if (File.Exists(fileName)) + { + return fileName; + } + + foreach (string directory in Directory.GetDirectories(".", "*", SearchOption.TopDirectoryOnly)) + { + string candidate = Path.Combine(directory, fileName); + if (File.Exists(candidate)) + { + return candidate; + } + } + + return null; + } + } +} diff --git a/actions/docs-verifier/MSDocsBuildVerifier.sln b/actions/docs-verifier/MSDocsBuildVerifier.sln index ad8db2a0..54c71568 100644 --- a/actions/docs-verifier/MSDocsBuildVerifier.sln +++ b/actions/docs-verifier/MSDocsBuildVerifier.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio Version 17 -VisualStudioVersion = 17.5.33402.96 +# Visual Studio Version 18 +VisualStudioVersion = 18.9.12128.139 oobstable MinimumVisualStudioVersion = 15.0.26124.0 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{FB9E2FB6-DF28-4985-87D6-0334B8260365}" ProjectSection(SolutionItems) = preProject @@ -28,6 +28,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ActionRunner", "src\ActionR EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "BuildVerifier.IO.Abstractions", "src\BuildVerifier.IO.Abstractions\BuildVerifier.IO.Abstractions.csproj", "{FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "DocfxVerifier", "DocfxVerifier\DocfxVerifier.csproj", "{AE4A266D-7C89-42A3-BA85-E723E80E3A24}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -122,6 +124,18 @@ Global {FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}.Release|x64.Build.0 = Release|Any CPU {FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}.Release|x86.ActiveCfg = Release|Any CPU {FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}.Release|x86.Build.0 = Release|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|Any CPU.Build.0 = Debug|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x64.ActiveCfg = Debug|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x64.Build.0 = Debug|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x86.ActiveCfg = Debug|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x86.Build.0 = Debug|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|Any CPU.ActiveCfg = Release|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|Any CPU.Build.0 = Release|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x64.ActiveCfg = Release|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x64.Build.0 = Release|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x86.ActiveCfg = Release|Any CPU + {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj b/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj index 551ad3db..227d485e 100644 --- a/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj +++ b/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj @@ -8,6 +8,7 @@ + diff --git a/actions/docs-verifier/src/ActionRunner/Program.cs b/actions/docs-verifier/src/ActionRunner/Program.cs index bd966d0d..116aedb8 100644 --- a/actions/docs-verifier/src/ActionRunner/Program.cs +++ b/actions/docs-verifier/src/ActionRunner/Program.cs @@ -106,6 +106,31 @@ IEnumerable matchers = await docfxConfigurationReader.MapConfigurationAsync(); IEnumerable pullRequestFiles = await GitHubPullRequest.GetPullRequestFilesAsync(pullRequestNumber); +// Check docfx.json for invalid paths. +if (pullRequestFiles.Any(IsDocfxConfigurationChange) + && !await DocfxVerifier.PathVerifier.WriteResultsAsync(Console.Out)) +{ + returnCode++; +} + +// Verify that all redirection URLs in modified +// redirection files are valid and reachable. +HashSet redirectionFileSet = + new(redirectionFiles.Select(NormalizePath), StringComparer.OrdinalIgnoreCase); + +IEnumerable modifiedRedirectionFiles = pullRequestFiles + .Where(file => !file.IsRemoved() && IsRegisteredRedirectionFile(file.FileName, redirectionFileSet)) + .Select(file => file.FileName) + .Distinct(StringComparer.OrdinalIgnoreCase); + +foreach (string redirectionFilePath in modifiedRedirectionFiles) +{ + if (!await RedirectTargetVerifier.WriteResultsAsync(Console.Out, redirectionFilePath)) + { + returnCode++; + } +} + List files = [.. pullRequestFiles.Where(f => IsRedirectableFile(f, matchers))]; @@ -132,8 +157,7 @@ return returnCode; -static bool IsRedirectableFile( - PullRequestFile file, IEnumerable matchers) +static bool IsRedirectableFile(PullRequestFile file, IEnumerable matchers) { string? deletedFileName = file.IsRenamed() ? file.PreviousFileName @@ -144,15 +168,27 @@ static bool IsRedirectableFile( // A deleted toc.yml doesn't need redirection. // Also, don't require a redirection for file patterns specified as "exclude"s in docfx config file. - return !isDeletedToc && IsYmlOrMarkdownFile(deletedFileName) + return !isDeletedToc + && IsYmlOrMarkdownFile(deletedFileName) && matchers.Any(m => m.Match(deletedFileName).HasMatches); } static bool IsYmlOrMarkdownFile([NotNullWhen(true)] string? fileName) => Path.GetExtension(fileName) is ".yml" or ".md"; +static bool IsDocfxConfigurationChange(PullRequestFile file) => + IsDocfxJsonPath(file.FileName) || IsDocfxJsonPath(file.PreviousFileName); + +static bool IsDocfxJsonPath(string? path) => + path is not null && + path.Replace('\\', '/').EndsWith("docfx.json", StringComparison.OrdinalIgnoreCase); + +static bool IsRegisteredRedirectionFile(string? path, HashSet redirectionFilesSet) => + path is not null && redirectionFilesSet.Contains(NormalizePath(path)); + +static string NormalizePath(string path) => path.Replace('\\', '/'); + static bool IsExtensionChangeOnly(string file1, string file2) => RemoveExtension(file1).Equals(RemoveExtension(file2), StringComparison.OrdinalIgnoreCase); -static string RemoveExtension(string file) => - file.Substring(0, file.Length - Path.GetExtension(file).Length); +static string RemoveExtension(string file) => file[..^Path.GetExtension(file).Length]; diff --git a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs new file mode 100644 index 00000000..5c1be7ca --- /dev/null +++ b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs @@ -0,0 +1,105 @@ +using System.Collections.Immutable; +using System.Net; + +namespace RedirectionVerifier; + +public static class RedirectTargetVerifier +{ + private const string LearnMicrosoftCom = "https://learn.microsoft.com"; + private static readonly HttpClient s_httpClient = new() + { + Timeout = TimeSpan.FromSeconds(15) + }; + + /// + /// Verifies that redirect targets in an entire redirection file are valid. + /// + public static async Task WriteResultsAsync( + TextWriter writer, + string redirectionFilePath) + { + ArgumentNullException.ThrowIfNull(writer, nameof(writer)); + ArgumentNullException.ThrowIfNull(redirectionFilePath, nameof(redirectionFilePath)); + + if (!File.Exists(redirectionFilePath)) + { + await writer.WriteLineAsync($"::error::Redirection file '{redirectionFilePath}' does not exist."); + return false; + } + + OpenPublishingRedirectionReader reader = new(redirectionFilePath); + ImmutableArray redirections = await reader.MapConfigurationAsync(); + if (redirections.IsDefaultOrEmpty) + { + return true; + } + + bool isValid = true; + for (int i = 0; i < redirections.Length; i++) + { + string? redirectUrl = redirections[i].RedirectUrl; + if (string.IsNullOrWhiteSpace(redirectUrl)) + { + await writer.WriteLineAsync($"::error file={redirectionFilePath}::Redirection at index {i} has an empty 'redirect_url'."); + isValid = false; + continue; + } + + string redirectTarget = redirectUrl.StartsWith('/') + ? $"{LearnMicrosoftCom}{redirectUrl}" + : redirectUrl; + + bool hasValidUri = Uri.TryCreate(redirectTarget, UriKind.Absolute, out Uri? uri) + && uri is not null + && (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps); + + if (!hasValidUri) + { + await writer.WriteLineAsync($"::error file={redirectionFilePath}::Invalid 'redirect_url' at index {i}: '{redirectUrl}'."); + isValid = false; + continue; + } + + HttpStatusCode? statusCode = await GetStatusCodeAsync(uri!); + if (statusCode is null) + { + await writer.WriteLineAsync($"::error file={redirectionFilePath}::Unable to verify 'redirect_url' at index {i}: '{redirectUrl}'."); + isValid = false; + continue; + } + + if (statusCode == HttpStatusCode.NotFound) + { + await writer.WriteLineAsync($"::error file={redirectionFilePath}::Redirect target returns 404 at index {i}: '{redirectUrl}'."); + isValid = false; + } + } + + return isValid; + } + + private static async Task GetStatusCodeAsync(Uri uri) + { + try + { + using HttpRequestMessage headRequest = new(HttpMethod.Head, uri); + using HttpResponseMessage headResponse = await s_httpClient.SendAsync(headRequest); + if (headResponse.StatusCode is HttpStatusCode.MethodNotAllowed or HttpStatusCode.NotImplemented) + { + using HttpRequestMessage getRequest = new(HttpMethod.Get, uri); + using HttpResponseMessage getResponse = await s_httpClient.SendAsync(getRequest, HttpCompletionOption.ResponseHeadersRead); + return getResponse.StatusCode; + } + + return headResponse.StatusCode; + } + catch (HttpRequestException) + { + return null; + } + catch (TaskCanceledException) + { + return null; + } + } +} diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj b/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj index 14fc2bce..323ba0c0 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj +++ b/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj @@ -19,6 +19,8 @@ + + - \ No newline at end of file + diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs new file mode 100644 index 00000000..5560d546 --- /dev/null +++ b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs @@ -0,0 +1,136 @@ +using DocfxVerifier; +using Xunit; + +namespace GitHub.UnitTests; + +public class PathVerifierTests +{ + private static readonly SemaphoreSlim s_currentDirectoryLock = new(1, 1); + + [Fact] + public async Task WriteResultsAsyncReturnsTrueForValidPaths() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory("docs"); + Directory.CreateDirectory(Path.Combine("templates", "default")); + await File.WriteAllTextAsync("docfx.json", """ + { + "build": { + "content": [ + { + "files": ["**/*.md", "**/*.yml"], + "src": "docs", + "exclude": ["**/includes/**"] + } + ], + "template": ["templates/default"], + "xref": ["https://learn.microsoft.com/dotnet/.xrefmap.json"] + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + [Fact] + public async Task WriteResultsAsyncReturnsFalseForInvalidPaths() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + await File.WriteAllTextAsync("docfx.json", """ + { + "build": { + "content": [ + { + "files": ["**/*.md"], + "src": "missing-folder" + } + ] + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer); + + Assert.False(result); + Assert.Contains("Path 'missing-folder' is invalid", writer.ToString(), StringComparison.Ordinal); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + [Fact] + public async Task WriteResultsAsyncFindsDocfxInSubdirectory() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory("docs"); + Directory.CreateDirectory("docs/content"); + Directory.CreateDirectory(Path.Combine("docs", "templates", "default")); + await File.WriteAllTextAsync(Path.Combine("docs", "docfx.json"), """ + { + "build": { + "content": [ + { + "files": ["**/*.md"], + "src": "content" + } + ], + "template": ["templates/default"] + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + private static string CreateTempDirectory() + { + string path = Path.Combine(Path.GetTempPath(), $"path-verifier-tests-{Guid.NewGuid():N}"); + Directory.CreateDirectory(path); + return path; + } +} diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs new file mode 100644 index 00000000..c0efb709 --- /dev/null +++ b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs @@ -0,0 +1,157 @@ +using System.Net; +using System.Net.Sockets; +using RedirectionVerifier; +using Xunit; + +namespace GitHub.UnitTests; + +public class RedirectTargetVerifierTests +{ + [Fact] + public async Task WriteResultsAsyncReturnsTrueForValidUrl() + { + await using var server = new TestHttpServer(new Dictionary + { + ["/ok"] = HttpStatusCode.OK + }); + + string redirectionFilePath = await CreateRedirectionFileAsync($"{server.BaseUrl}/ok"); + try + { + using var writer = new StringWriter(); + bool result = await RedirectTargetVerifier.WriteResultsAsync(writer, redirectionFilePath); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + File.Delete(redirectionFilePath); + } + } + + [Fact] + public async Task WriteResultsAsyncReturnsFalseForInvalidUrl() + { + string redirectionFilePath = await CreateRedirectionFileAsync("not-a-valid-url"); + try + { + using var writer = new StringWriter(); + bool result = await RedirectTargetVerifier.WriteResultsAsync(writer, redirectionFilePath); + + Assert.False(result); + Assert.Contains("Invalid 'redirect_url'", writer.ToString(), StringComparison.Ordinal); + } + finally + { + File.Delete(redirectionFilePath); + } + } + + [Fact] + public async Task WriteResultsAsyncReturnsFalseFor404Url() + { + await using var server = new TestHttpServer(new Dictionary + { + ["/missing"] = HttpStatusCode.NotFound + }); + + string redirectionFilePath = await CreateRedirectionFileAsync($"{server.BaseUrl}/missing"); + try + { + using var writer = new StringWriter(); + bool result = await RedirectTargetVerifier.WriteResultsAsync(writer, redirectionFilePath); + + Assert.False(result); + Assert.Contains("returns 404", writer.ToString(), StringComparison.Ordinal); + } + finally + { + File.Delete(redirectionFilePath); + } + } + + private static async Task CreateRedirectionFileAsync(string redirectUrl) + { + string filePath = Path.Combine(Path.GetTempPath(), $"redirect-{Guid.NewGuid():N}.json"); + string content = $$""" + { + "redirections": [ + { + "source_path": "docs/old.md", + "redirect_url": "{{redirectUrl}}" + } + ] + } + """; + + await File.WriteAllTextAsync(filePath, content); + return filePath; + } + + private sealed class TestHttpServer : IAsyncDisposable + { + private readonly HttpListener _listener; + private readonly Task _listenerTask; + private readonly Dictionary _responses; + + public TestHttpServer(Dictionary responses) + { + _responses = responses; + int port = GetFreePort(); + BaseUrl = $"http://127.0.0.1:{port}"; + + _listener = new HttpListener(); + _listener.Prefixes.Add($"{BaseUrl}/"); + _listener.Start(); + + _listenerTask = Task.Run(HandleRequestsAsync); + } + + public string BaseUrl { get; } + + private async Task HandleRequestsAsync() + { + while (_listener.IsListening) + { + HttpListenerContext? context; + try + { + context = await _listener.GetContextAsync(); + } + catch (HttpListenerException) + { + break; + } + catch (ObjectDisposedException) + { + break; + } + + string path = context.Request.Url?.AbsolutePath ?? "/"; + HttpStatusCode statusCode = _responses.TryGetValue(path, out HttpStatusCode configured) + ? configured + : HttpStatusCode.OK; + + context.Response.StatusCode = (int)statusCode; + context.Response.ContentLength64 = 0; + context.Response.Close(); + } + } + + public async ValueTask DisposeAsync() + { + _listener.Stop(); + _listener.Close(); + await _listenerTask; + } + + private static int GetFreePort() + { + using var tcpListener = new TcpListener(IPAddress.Loopback, 0); + tcpListener.Start(); + int port = ((IPEndPoint)tcpListener.LocalEndpoint).Port; + return port; + } + } +} From 8efb65e07ff9273b6aad838b5d73825e440efeab Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:00:17 -0700 Subject: [PATCH 2/8] add more tests --- .../DocfxVerifier/PathVerifier.cs | 22 ++++++++- .../GitHub.UnitTests/PathVerifierTests.cs | 47 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/actions/docs-verifier/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/DocfxVerifier/PathVerifier.cs index 0a64e71b..5ffa11ab 100644 --- a/actions/docs-verifier/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/DocfxVerifier/PathVerifier.cs @@ -83,7 +83,27 @@ private static void ValidateElement( ValidateElement(property.Value, property.Name, childPath, repositoryRoot, configurationDirectory, errors); } - if (propertyName is not null && s_pathObjectKeyPropertyNames.Contains(propertyName)) + if (string.Equals(propertyName, "fileMetadata", StringComparison.Ordinal)) + { + foreach (JsonProperty metadataProperty in element.EnumerateObject()) + { + if (metadataProperty.Value.ValueKind != JsonValueKind.Object) + { + continue; + } + + foreach (JsonProperty pathProperty in metadataProperty.Value.EnumerateObject()) + { + ValidatePath( + pathProperty.Name, + $"{jsonPath}.{metadataProperty.Name}.{pathProperty.Name}", + repositoryRoot, + configurationDirectory, + errors); + } + } + } + else if (propertyName is not null && s_pathObjectKeyPropertyNames.Contains(propertyName)) { foreach (JsonProperty property in element.EnumerateObject()) { diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs index 5560d546..ce8fdae1 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs @@ -49,6 +49,53 @@ await File.WriteAllTextAsync("docfx.json", """ } } + [Fact] + public async Task WriteResultsAsyncValidatesFileMetadataPaths() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory(Path.Combine("docs", "valid")); + + await File.WriteAllTextAsync("docfx.json", """ + { + "build": { + "content": [ + { + "files": ["**/*.md"], + "src": "docs" + } + ], + "fileMetadata": { + "ms.author": { + "docs/valid/**/**.{md,yml}": "someone", + "missing/path/**/**.{md,yml}": "someone" + } + } + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer); + string output = writer.ToString(); + + Assert.False(result); + Assert.Contains("Path 'missing/path/**/**.{md,yml}' is invalid", output, StringComparison.Ordinal); + Assert.DoesNotContain("Path 'docs/valid/**/**.{md,yml}' is invalid", output, StringComparison.Ordinal); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + [Fact] public async Task WriteResultsAsyncReturnsFalseForInvalidPaths() { From 26810f60e6443b29b176aa8b1d5565b8f55b3844 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:19:43 -0700 Subject: [PATCH 3/8] respond to feedback --- actions/docs-verifier/MSDocsBuildVerifier.sln | 28 +++++----- .../src/ActionRunner/ActionRunner.csproj | 2 +- .../docs-verifier/src/ActionRunner/Program.cs | 17 +++--- .../DocfxVerifier/DocfxVerifier.csproj | 0 .../{ => src}/DocfxVerifier/PathVerifier.cs | 16 +++++- .../GitHub.UnitTests/GitHub.UnitTests.csproj | 2 +- .../GitHub.UnitTests/PathVerifierTests.cs | 54 +++++++++++++++++++ 7 files changed, 94 insertions(+), 25 deletions(-) rename actions/docs-verifier/{ => src}/DocfxVerifier/DocfxVerifier.csproj (100%) rename actions/docs-verifier/{ => src}/DocfxVerifier/PathVerifier.cs (93%) diff --git a/actions/docs-verifier/MSDocsBuildVerifier.sln b/actions/docs-verifier/MSDocsBuildVerifier.sln index 54c71568..daee2e52 100644 --- a/actions/docs-verifier/MSDocsBuildVerifier.sln +++ b/actions/docs-verifier/MSDocsBuildVerifier.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio Version 18 -VisualStudioVersion = 18.9.12128.139 oobstable +VisualStudioVersion = 18.9.12128.139 MinimumVisualStudioVersion = 15.0.26124.0 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{FB9E2FB6-DF28-4985-87D6-0334B8260365}" ProjectSection(SolutionItems) = preProject @@ -28,7 +28,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ActionRunner", "src\ActionR EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "BuildVerifier.IO.Abstractions", "src\BuildVerifier.IO.Abstractions\BuildVerifier.IO.Abstractions.csproj", "{FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "DocfxVerifier", "DocfxVerifier\DocfxVerifier.csproj", "{AE4A266D-7C89-42A3-BA85-E723E80E3A24}" +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "DocfxVerifier", "src\DocfxVerifier\DocfxVerifier.csproj", "{3E0FB4E3-3080-D891-0CB6-382B700A7AA8}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -124,18 +124,18 @@ Global {FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}.Release|x64.Build.0 = Release|Any CPU {FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}.Release|x86.ActiveCfg = Release|Any CPU {FD67DE5D-D8C0-48AD-A7F9-E6F316821E10}.Release|x86.Build.0 = Release|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|Any CPU.Build.0 = Debug|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x64.ActiveCfg = Debug|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x64.Build.0 = Debug|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x86.ActiveCfg = Debug|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Debug|x86.Build.0 = Debug|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|Any CPU.ActiveCfg = Release|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|Any CPU.Build.0 = Release|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x64.ActiveCfg = Release|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x64.Build.0 = Release|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x86.ActiveCfg = Release|Any CPU - {AE4A266D-7C89-42A3-BA85-E723E80E3A24}.Release|x86.Build.0 = Release|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Debug|Any CPU.Build.0 = Debug|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Debug|x64.ActiveCfg = Debug|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Debug|x64.Build.0 = Debug|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Debug|x86.ActiveCfg = Debug|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Debug|x86.Build.0 = Debug|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Release|Any CPU.ActiveCfg = Release|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Release|Any CPU.Build.0 = Release|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Release|x64.ActiveCfg = Release|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Release|x64.Build.0 = Release|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Release|x86.ActiveCfg = Release|Any CPU + {3E0FB4E3-3080-D891-0CB6-382B700A7AA8}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj b/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj index 227d485e..935fa217 100644 --- a/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj +++ b/actions/docs-verifier/src/ActionRunner/ActionRunner.csproj @@ -8,7 +8,7 @@ - + diff --git a/actions/docs-verifier/src/ActionRunner/Program.cs b/actions/docs-verifier/src/ActionRunner/Program.cs index 116aedb8..ccb0e1df 100644 --- a/actions/docs-verifier/src/ActionRunner/Program.cs +++ b/actions/docs-verifier/src/ActionRunner/Program.cs @@ -106,11 +106,17 @@ IEnumerable matchers = await docfxConfigurationReader.MapConfigurationAsync(); IEnumerable pullRequestFiles = await GitHubPullRequest.GetPullRequestFilesAsync(pullRequestNumber); -// Check docfx.json for invalid paths. -if (pullRequestFiles.Any(IsDocfxConfigurationChange) - && !await DocfxVerifier.PathVerifier.WriteResultsAsync(Console.Out)) +IEnumerable modifiedDocfxFiles = pullRequestFiles + .Where(file => !file.IsRemoved() && IsDocfxJsonPath(file.FileName)) + .Select(file => file.FileName) + .Distinct(StringComparer.OrdinalIgnoreCase); + +foreach (string docfxFilePath in modifiedDocfxFiles) { - returnCode++; + if (!await DocfxVerifier.PathVerifier.WriteResultsAsync(Console.Out, docfxFilePath)) + { + returnCode++; + } } // Verify that all redirection URLs in modified @@ -176,9 +182,6 @@ static bool IsRedirectableFile(PullRequestFile file, IEnumerable matche static bool IsYmlOrMarkdownFile([NotNullWhen(true)] string? fileName) => Path.GetExtension(fileName) is ".yml" or ".md"; -static bool IsDocfxConfigurationChange(PullRequestFile file) => - IsDocfxJsonPath(file.FileName) || IsDocfxJsonPath(file.PreviousFileName); - static bool IsDocfxJsonPath(string? path) => path is not null && path.Replace('\\', '/').EndsWith("docfx.json", StringComparison.OrdinalIgnoreCase); diff --git a/actions/docs-verifier/DocfxVerifier/DocfxVerifier.csproj b/actions/docs-verifier/src/DocfxVerifier/DocfxVerifier.csproj similarity index 100% rename from actions/docs-verifier/DocfxVerifier/DocfxVerifier.csproj rename to actions/docs-verifier/src/DocfxVerifier/DocfxVerifier.csproj diff --git a/actions/docs-verifier/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs similarity index 93% rename from actions/docs-verifier/DocfxVerifier/PathVerifier.cs rename to actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs index 5ffa11ab..b2d96012 100644 --- a/actions/docs-verifier/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs @@ -38,17 +38,29 @@ public static class PathVerifier /// /// Verifies that file paths in the docfx.json file are valid. /// - public static async Task WriteResultsAsync(TextWriter writer) + public static Task WriteResultsAsync(TextWriter writer) => + WriteResultsAsync(writer, configurationPath: null); + + /// + /// Verifies that file paths in a specific docfx.json file are valid. + /// + public static async Task WriteResultsAsync(TextWriter writer, string? configurationPath) { ArgumentNullException.ThrowIfNull(writer, nameof(writer)); - string? configurationPath = FindDocfxConfigurationPath(); + configurationPath ??= FindDocfxConfigurationPath(); if (configurationPath is null) { await writer.WriteLineAsync("::error::Unable to find docfx.json in the repository root or its immediate subdirectories."); return false; } + if (!File.Exists(configurationPath)) + { + await writer.WriteLineAsync($"::error::docfx.json file '{configurationPath}' does not exist."); + return false; + } + using FileStream stream = File.OpenRead(configurationPath); using JsonDocument json = await JsonDocument.ParseAsync(stream); diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj b/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj index 323ba0c0..74c7e150 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj +++ b/actions/docs-verifier/tests/GitHub.UnitTests/GitHub.UnitTests.csproj @@ -19,7 +19,7 @@ - + diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs index ce8fdae1..6875d94e 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs @@ -174,6 +174,60 @@ await File.WriteAllTextAsync(Path.Combine("docs", "docfx.json"), """ } } + [Fact] + public async Task WriteResultsAsyncUsesSpecifiedDocfxPath() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + + Directory.CreateDirectory("valid-docs"); + + await File.WriteAllTextAsync("docfx.json", """ + { + "build": { + "content": [ + { + "files": ["**/*.md"], + "src": "missing-root-folder" + } + ] + } + } + """); + + string modifiedDocfxPath = Path.Combine("valid-docs", "docfx.json"); + await File.WriteAllTextAsync(modifiedDocfxPath, """ + { + "build": { + "content": [ + { + "files": ["**/*.md"], + "src": "." + } + ] + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer, modifiedDocfxPath); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + private static string CreateTempDirectory() { string path = Path.Combine(Path.GetTempPath(), $"path-verifier-tests-{Guid.NewGuid():N}"); From 29ca5e28598513e146cd58a19f62aea9747ecf24 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:31:43 -0700 Subject: [PATCH 4/8] more feedback --- actions/docs-verifier/MSDocsBuildVerifier.sln | 2 +- actions/docs-verifier/src/ActionRunner/Program.cs | 14 +++++++++++--- .../src/DocfxVerifier/PathVerifier.cs | 13 ++++++++++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/actions/docs-verifier/MSDocsBuildVerifier.sln b/actions/docs-verifier/MSDocsBuildVerifier.sln index daee2e52..7c0d8899 100644 --- a/actions/docs-verifier/MSDocsBuildVerifier.sln +++ b/actions/docs-verifier/MSDocsBuildVerifier.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio Version 18 -VisualStudioVersion = 18.9.12128.139 +VisualStudioVersion = 18.9.12128.139 oobstable MinimumVisualStudioVersion = 15.0.26124.0 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{FB9E2FB6-DF28-4985-87D6-0334B8260365}" ProjectSection(SolutionItems) = preProject diff --git a/actions/docs-verifier/src/ActionRunner/Program.cs b/actions/docs-verifier/src/ActionRunner/Program.cs index ccb0e1df..316e6763 100644 --- a/actions/docs-verifier/src/ActionRunner/Program.cs +++ b/actions/docs-verifier/src/ActionRunner/Program.cs @@ -182,9 +182,17 @@ static bool IsRedirectableFile(PullRequestFile file, IEnumerable matche static bool IsYmlOrMarkdownFile([NotNullWhen(true)] string? fileName) => Path.GetExtension(fileName) is ".yml" or ".md"; -static bool IsDocfxJsonPath(string? path) => - path is not null && - path.Replace('\\', '/').EndsWith("docfx.json", StringComparison.OrdinalIgnoreCase); +static bool IsDocfxJsonPath(string? path) +{ + if (path is null) + { + return false; + } + + string normalized = path.Replace('\\', '/'); + return normalized.Equals("docfx.json", StringComparison.OrdinalIgnoreCase) + || normalized.EndsWith("/docfx.json", StringComparison.OrdinalIgnoreCase); +} static bool IsRegisteredRedirectionFile(string? path, HashSet redirectionFilesSet) => path is not null && redirectionFilesSet.Contains(NormalizePath(path)); diff --git a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs index b2d96012..5d4d3a6f 100644 --- a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs @@ -174,7 +174,9 @@ private static void ValidatePath( return; } - if (Uri.TryCreate(path, UriKind.Absolute, out _)) + if (Uri.TryCreate(path, UriKind.Absolute, out Uri? uri) + && uri is not null + && (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps)) { return; } @@ -197,6 +199,15 @@ private static void ValidatePath( private static bool ExistsInRepository(string baseDirectory, string path) { string combinedPath = Path.GetFullPath(Path.Combine(baseDirectory, path)); + string relative = Path.GetRelativePath(baseDirectory, combinedPath); + + if (relative.Equals("..", StringComparison.Ordinal) + || relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) + || relative.StartsWith(".." + Path.AltDirectorySeparatorChar, StringComparison.Ordinal)) + { + return false; + } + return File.Exists(combinedPath) || Directory.Exists(combinedPath); } From fe4b6f06919f7e7e3174a4b96332e159ea7148f2 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:41:15 -0700 Subject: [PATCH 5/8] Apply batched suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs | 1 - .../src/RedirectionVerifier/RedirectTargetVerifier.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs index 5d4d3a6f..fd5659a7 100644 --- a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs @@ -10,7 +10,6 @@ public static class PathVerifier private static readonly HashSet s_pathStringPropertyNames = [ "src", - "dest" ]; private static readonly HashSet s_pathArrayPropertyNames = diff --git a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs index 5c1be7ca..fff72466 100644 --- a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs +++ b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs @@ -84,7 +84,7 @@ public static async Task WriteResultsAsync( { using HttpRequestMessage headRequest = new(HttpMethod.Head, uri); using HttpResponseMessage headResponse = await s_httpClient.SendAsync(headRequest); - if (headResponse.StatusCode is HttpStatusCode.MethodNotAllowed or HttpStatusCode.NotImplemented) + if (headResponse.StatusCode is HttpStatusCode.MethodNotAllowed or HttpStatusCode.NotImplemented or HttpStatusCode.NotFound) { using HttpRequestMessage getRequest = new(HttpMethod.Get, uri); using HttpResponseMessage getResponse = await s_httpClient.SendAsync(getRequest, HttpCompletionOption.ResponseHeadersRead); From ec5e4a7ec5d9feae827512fa5f72a64acabaefd8 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:08:37 -0700 Subject: [PATCH 6/8] respond to feedback --- .../src/DocfxVerifier/PathVerifier.cs | 303 ++++++++++++------ .../Properties/AssemblyInfo.cs | 3 + .../RedirectTargetVerifier.cs | 75 ++++- .../GitHub.UnitTests/PathVerifierTests.cs | 208 ++++++++++++ .../RedirectTargetVerifierTests.cs | 116 +++---- 5 files changed, 524 insertions(+), 181 deletions(-) create mode 100644 actions/docs-verifier/src/RedirectionVerifier/Properties/AssemblyInfo.cs diff --git a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs index 5d4d3a6f..95f0b296 100644 --- a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs @@ -7,33 +7,10 @@ namespace DocfxVerifier /// public static class PathVerifier { - private static readonly HashSet s_pathStringPropertyNames = - [ - "src", - "dest" - ]; - - private static readonly HashSet s_pathArrayPropertyNames = - [ - "files", - "exclude", - "xref", - "template", - "resource", - "overwrite", - "globalMetadataFiles", - "fileMetadataFiles" - ]; - - private static readonly HashSet s_pathObjectKeyPropertyNames = - [ - "fileMetadata", - "open_to_public_contributors", - "ms.collection", - "ms.custom", - "ms.update-cycle", - "no-loc" - ]; + private static readonly JsonDocumentOptions s_jsonDocumentOptions = new() + { + AllowTrailingCommas = true + }; /// /// Verifies that file paths in the docfx.json file are valid. @@ -62,14 +39,14 @@ public static async Task WriteResultsAsync(TextWriter writer, string? conf } using FileStream stream = File.OpenRead(configurationPath); - using JsonDocument json = await JsonDocument.ParseAsync(stream); + using JsonDocument json = await JsonDocument.ParseAsync(stream, s_jsonDocumentOptions); string repositoryRoot = Directory.GetCurrentDirectory(); string configurationDirectory = Path.GetDirectoryName(Path.GetFullPath(configurationPath)) ?? repositoryRoot; string configurationPathForLog = configurationPath.Replace('\\', '/'); - var errors = new List(); - ValidateElement(json.RootElement, null, "$", repositoryRoot, configurationDirectory, errors); + var errors = new List(); + ValidateConfiguration(json.RootElement, repositoryRoot, configurationDirectory, errors); foreach (string error in errors) { @@ -79,94 +56,206 @@ public static async Task WriteResultsAsync(TextWriter writer, string? conf return errors.Count == 0; } - private static void ValidateElement( + private static void ValidateConfiguration( JsonElement element, - string? propertyName, - string jsonPath, string repositoryRoot, string configurationDirectory, List errors) { - if (element.ValueKind == JsonValueKind.Object) + if (element.ValueKind != JsonValueKind.Object) { - foreach (JsonProperty property in element.EnumerateObject()) - { - string childPath = $"{jsonPath}.{property.Name}"; - ValidateElement(property.Value, property.Name, childPath, repositoryRoot, configurationDirectory, errors); - } + return; + } - if (string.Equals(propertyName, "fileMetadata", StringComparison.Ordinal)) - { - foreach (JsonProperty metadataProperty in element.EnumerateObject()) - { - if (metadataProperty.Value.ValueKind != JsonValueKind.Object) - { - continue; - } - - foreach (JsonProperty pathProperty in metadataProperty.Value.EnumerateObject()) - { - ValidatePath( - pathProperty.Name, - $"{jsonPath}.{metadataProperty.Name}.{pathProperty.Name}", - repositoryRoot, - configurationDirectory, - errors); - } - } - } - else if (propertyName is not null && s_pathObjectKeyPropertyNames.Contains(propertyName)) - { - foreach (JsonProperty property in element.EnumerateObject()) - { - ValidatePath(property.Name, $"{jsonPath}.{property.Name}", repositoryRoot, configurationDirectory, errors); - } - } + if (element.TryGetProperty("build", out JsonElement buildSection) + && buildSection.ValueKind == JsonValueKind.Object) + { + ValidateBuildSection(buildSection, "$.build", repositoryRoot, configurationDirectory, errors); + } - return; + if (element.TryGetProperty("metadata", out JsonElement metadataSection)) + { + ValidateFileMappingArray(metadataSection, "$.metadata", repositoryRoot, configurationDirectory, configurationDirectory, errors); } + } - if (element.ValueKind == JsonValueKind.Array) + private static void ValidateBuildSection( + JsonElement buildSection, + string jsonPath, + string repositoryRoot, + string configurationDirectory, + List errors) + { + ValidateStringProperty(buildSection, "dest", $"{jsonPath}.dest", repositoryRoot, configurationDirectory, errors); + ValidateStringArrayProperty(buildSection, "template", $"{jsonPath}.template", repositoryRoot, configurationDirectory, errors); + ValidateStringArrayProperty(buildSection, "xref", $"{jsonPath}.xref", repositoryRoot, configurationDirectory, errors); + ValidateStringArrayProperty(buildSection, "globalMetadataFiles", $"{jsonPath}.globalMetadataFiles", repositoryRoot, configurationDirectory, errors); + ValidateStringArrayProperty(buildSection, "fileMetadataFiles", $"{jsonPath}.fileMetadataFiles", repositoryRoot, configurationDirectory, errors); + + ValidateFileMappingArrayProperty(buildSection, "content", $"{jsonPath}.content", repositoryRoot, configurationDirectory, errors); + ValidateFileMappingArrayProperty(buildSection, "resource", $"{jsonPath}.resource", repositoryRoot, configurationDirectory, errors); + ValidateFileMappingArrayProperty(buildSection, "overwrite", $"{jsonPath}.overwrite", repositoryRoot, configurationDirectory, errors); + + if (buildSection.TryGetProperty("fileMetadata", out JsonElement fileMetadata) + && fileMetadata.ValueKind == JsonValueKind.Object) + { + foreach (JsonProperty metadataProperty in fileMetadata.EnumerateObject()) { - if (propertyName is not null && s_pathArrayPropertyNames.Contains(propertyName)) + if (metadataProperty.Value.ValueKind != JsonValueKind.Object) { - int index = 0; - foreach (JsonElement item in element.EnumerateArray()) - { - if (item.ValueKind == JsonValueKind.String) - { - ValidatePath(item.GetString(), $"{jsonPath}[{index}]", repositoryRoot, configurationDirectory, errors); - } - - index++; - } + continue; } - else + + foreach (JsonProperty pathProperty in metadataProperty.Value.EnumerateObject()) { - int index = 0; - foreach (JsonElement item in element.EnumerateArray()) - { - ValidateElement(item, propertyName, $"{jsonPath}[{index}]", repositoryRoot, configurationDirectory, errors); - index++; - } + ValidatePath( + pathProperty.Name, + $"{jsonPath}.fileMetadata.{metadataProperty.Name}.{pathProperty.Name}", + repositoryRoot, + configurationDirectory, + errors); } + } + } + } - return; + private static void ValidateStringProperty( + JsonElement parent, + string propertyName, + string jsonPath, + string repositoryRoot, + string resolutionBaseDirectory, + List errors) + { + if (parent.TryGetProperty(propertyName, out JsonElement property) + && property.ValueKind == JsonValueKind.String) + { + ValidatePath(property.GetString(), jsonPath, repositoryRoot, resolutionBaseDirectory, errors); + } + } + + private static void ValidateStringArrayProperty( + JsonElement parent, + string propertyName, + string jsonPath, + string repositoryRoot, + string resolutionBaseDirectory, + List errors) + { + if (parent.TryGetProperty(propertyName, out JsonElement property)) + { + ValidateStringArray(property, jsonPath, repositoryRoot, resolutionBaseDirectory, errors); + } + } + + private static void ValidateFileMappingArrayProperty( + JsonElement parent, + string propertyName, + string jsonPath, + string repositoryRoot, + string configurationDirectory, + List errors) + { + if (parent.TryGetProperty(propertyName, out JsonElement property)) + { + ValidateFileMappingArray(property, jsonPath, repositoryRoot, configurationDirectory, configurationDirectory, errors); + } + } + + private static void ValidateFileMappingArray( + JsonElement mappings, + string jsonPath, + string repositoryRoot, + string configurationDirectory, + string resolutionBaseDirectory, + List errors) + { + if (mappings.ValueKind != JsonValueKind.Array) + { + return; + } + + int index = 0; + foreach (JsonElement mapping in mappings.EnumerateArray()) + { + string mappingPath = $"{jsonPath}[{index}]"; + if (mapping.ValueKind == JsonValueKind.String) + { + ValidatePath(mapping.GetString(), mappingPath, repositoryRoot, resolutionBaseDirectory, errors); + } + else if (mapping.ValueKind == JsonValueKind.Object) + { + ValidateFileMappingObject(mapping, mappingPath, repositoryRoot, configurationDirectory, errors); + } + + index++; + } + } + + private static void ValidateFileMappingObject( + JsonElement mapping, + string jsonPath, + string repositoryRoot, + string configurationDirectory, + List errors) + { + string mappingSourceDirectory = configurationDirectory; + + if (mapping.TryGetProperty("src", out JsonElement src) + && src.ValueKind == JsonValueKind.String) + { + string? srcPath = src.GetString(); + ValidatePath(srcPath, $"{jsonPath}.src", repositoryRoot, configurationDirectory, errors); + + string? effectiveSourceDirectory = TryResolvePathWithinRepository(srcPath, repositoryRoot, configurationDirectory); + if (effectiveSourceDirectory is not null) + { + mappingSourceDirectory = effectiveSourceDirectory; } + } + + ValidateStringProperty(mapping, "dest", $"{jsonPath}.dest", repositoryRoot, configurationDirectory, errors); + + if (mapping.TryGetProperty("files", out JsonElement files)) + { + ValidateStringArray(files, $"{jsonPath}.files", repositoryRoot, mappingSourceDirectory, errors); + } + + if (mapping.TryGetProperty("exclude", out JsonElement exclude)) + { + ValidateStringArray(exclude, $"{jsonPath}.exclude", repositoryRoot, mappingSourceDirectory, errors); + } + } + + private static void ValidateStringArray( + JsonElement values, + string jsonPath, + string repositoryRoot, + string resolutionBaseDirectory, + List errors) + { + if (values.ValueKind != JsonValueKind.Array) + { + return; + } - if (element.ValueKind == JsonValueKind.String - && propertyName is not null - && s_pathStringPropertyNames.Contains(propertyName)) + int index = 0; + foreach (JsonElement item in values.EnumerateArray()) + { + if (item.ValueKind == JsonValueKind.String) { - ValidatePath(element.GetString(), jsonPath, repositoryRoot, configurationDirectory, errors); + ValidatePath(item.GetString(), $"{jsonPath}[{index}]", repositoryRoot, resolutionBaseDirectory, errors); } + + index++; } + } private static void ValidatePath( string? path, string jsonPath, string repositoryRoot, - string configurationDirectory, + string resolutionBaseDirectory, List errors) { if (string.IsNullOrWhiteSpace(path) || path is ".") @@ -188,27 +277,39 @@ private static void ValidatePath( return; } - bool existsRelativeToRoot = ExistsInRepository(repositoryRoot, nonWildcardPrefix); - bool existsRelativeToConfig = ExistsInRepository(configurationDirectory, nonWildcardPrefix); - if (!existsRelativeToRoot && !existsRelativeToConfig) + if (!ExistsInRepository(nonWildcardPrefix, repositoryRoot, resolutionBaseDirectory)) { - errors.Add($"{jsonPath}: Path '{path}' is invalid. Checked '{nonWildcardPrefix}' relative to repository root and config directory."); + errors.Add($"{jsonPath}: Path '{path}' is invalid."); } } - private static bool ExistsInRepository(string baseDirectory, string path) + private static bool ExistsInRepository(string path, string repositoryRoot, string resolutionBaseDirectory) + { + string? combinedPath = TryResolvePathWithinRepository(path, repositoryRoot, resolutionBaseDirectory); + return combinedPath is not null && (File.Exists(combinedPath) || Directory.Exists(combinedPath)); + } + + private static string? TryResolvePathWithinRepository( + string? path, + string repositoryRoot, + string resolutionBaseDirectory) + { + if (string.IsNullOrWhiteSpace(path)) { - string combinedPath = Path.GetFullPath(Path.Combine(baseDirectory, path)); - string relative = Path.GetRelativePath(baseDirectory, combinedPath); + return null; + } + + string combinedPath = Path.GetFullPath(Path.Combine(resolutionBaseDirectory, path)); + string relative = Path.GetRelativePath(Path.GetFullPath(repositoryRoot), combinedPath); if (relative.Equals("..", StringComparison.Ordinal) || relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || relative.StartsWith(".." + Path.AltDirectorySeparatorChar, StringComparison.Ordinal)) { - return false; + return null; } - return File.Exists(combinedPath) || Directory.Exists(combinedPath); + return combinedPath; } private static string GetNonWildcardPrefix(string path) diff --git a/actions/docs-verifier/src/RedirectionVerifier/Properties/AssemblyInfo.cs b/actions/docs-verifier/src/RedirectionVerifier/Properties/AssemblyInfo.cs new file mode 100644 index 00000000..542c99c6 --- /dev/null +++ b/actions/docs-verifier/src/RedirectionVerifier/Properties/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("GitHub.UnitTests")] diff --git a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs index 5c1be7ca..1df115eb 100644 --- a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs +++ b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs @@ -1,5 +1,6 @@ using System.Collections.Immutable; using System.Net; +using System.Net.Sockets; namespace RedirectionVerifier; @@ -17,9 +18,16 @@ public static class RedirectTargetVerifier public static async Task WriteResultsAsync( TextWriter writer, string redirectionFilePath) + => await WriteResultsAsync(writer, redirectionFilePath, GetStatusCodeAsync); + + internal static async Task WriteResultsAsync( + TextWriter writer, + string redirectionFilePath, + Func> statusCodeProvider) { ArgumentNullException.ThrowIfNull(writer, nameof(writer)); ArgumentNullException.ThrowIfNull(redirectionFilePath, nameof(redirectionFilePath)); + ArgumentNullException.ThrowIfNull(statusCodeProvider, nameof(statusCodeProvider)); if (!File.Exists(redirectionFilePath)) { @@ -60,7 +68,14 @@ public static async Task WriteResultsAsync( continue; } - HttpStatusCode? statusCode = await GetStatusCodeAsync(uri!); + if (!await IsPublicHttpTargetAsync(uri!)) + { + await writer.WriteLineAsync($"::error file={redirectionFilePath}::Disallowed 'redirect_url' target at index {i}: '{redirectUrl}'."); + isValid = false; + continue; + } + + HttpStatusCode? statusCode = await statusCodeProvider(uri!); if (statusCode is null) { await writer.WriteLineAsync($"::error file={redirectionFilePath}::Unable to verify 'redirect_url' at index {i}: '{redirectUrl}'."); @@ -78,6 +93,64 @@ public static async Task WriteResultsAsync( return isValid; } + private static async Task IsPublicHttpTargetAsync(Uri uri) + { + if (uri.IsLoopback) + { + return false; + } + + if (IPAddress.TryParse(uri.Host, out IPAddress? parsedAddress)) + { + return !IsPrivateOrLocalAddress(parsedAddress); + } + + try + { + IPAddress[] addresses = await Dns.GetHostAddressesAsync(uri.Host); + return addresses.All(address => !IsPrivateOrLocalAddress(address)); + } + catch (SocketException) + { + return true; + } + } + + private static bool IsPrivateOrLocalAddress(IPAddress address) + { + if (IPAddress.IsLoopback(address)) + { + return true; + } + + if (address.AddressFamily == AddressFamily.InterNetwork) + { + byte[] bytes = address.GetAddressBytes(); + return bytes[0] switch + { + 10 => true, + 127 => true, + 169 when bytes[1] == 254 => true, + 172 when bytes[1] >= 16 && bytes[1] <= 31 => true, + 192 when bytes[1] == 168 => true, + _ => false + }; + } + + if (address.AddressFamily == AddressFamily.InterNetworkV6) + { + if (address.IsIPv6LinkLocal || address.IsIPv6SiteLocal) + { + return true; + } + + byte[] bytes = address.GetAddressBytes(); + return (bytes[0] & 0xFE) == 0xFC; + } + + return false; + } + private static async Task GetStatusCodeAsync(Uri uri) { try diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs index 6875d94e..0574d0cf 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs @@ -228,6 +228,214 @@ await File.WriteAllTextAsync(modifiedDocfxPath, """ } } + [Fact] + public async Task WriteResultsAsyncValidatesFilesRelativeToMappingSrc() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory(Path.Combine("docs", "content", "guides")); + await File.WriteAllTextAsync(Path.Combine("docs", "content", "guides", "a.md"), "# title"); + + string configurationPath = Path.Combine("docs", "docfx.json"); + await File.WriteAllTextAsync(configurationPath, """ + { + "build": { + "content": [ + { + "src": "content", + "files": ["guides/a.md"] + } + ] + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer, configurationPath); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + [Fact] + public async Task WriteResultsAsyncAllowsParentRelativePathWhenInsideRepository() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory(Path.Combine("shared", "templates")); + Directory.CreateDirectory("docs"); + + string configurationPath = Path.Combine("docs", "docfx.json"); + await File.WriteAllTextAsync(configurationPath, """ + { + "build": { + "template": ["../shared/templates"] + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer, configurationPath); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + [Fact] + public async Task WriteResultsAsyncSupportsFileMappingShorthandAndObjectForms() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory(Path.Combine("docs", "articles")); + Directory.CreateDirectory(Path.Combine("docs", "assets", "images")); + Directory.CreateDirectory(Path.Combine("docs", "content", "overwrite")); + await File.WriteAllTextAsync(Path.Combine("docs", "assets", "images", "logo.png"), "binary"); + + string configurationPath = Path.Combine("docs", "docfx.json"); + await File.WriteAllTextAsync(configurationPath, """ + { + "build": { + "content": ["articles/**/*.md"], + "resource": [ + { + "src": "assets", + "files": ["images/logo.png"] + } + ], + "overwrite": [ + { + "src": "content", + "files": ["overwrite/**/*.md"] + } + ] + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer, configurationPath); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + [Fact] + public async Task WriteResultsAsyncIgnoresArbitraryMetadataPropertyNames() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory("docs"); + + await File.WriteAllTextAsync("docfx.json", """ + { + "build": { + "content": [ + { + "src": "docs", + "files": ["**/*.md"] + } + ] + }, + "globalMetadata": { + "src": "not-a-docfx-path" + } + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + + [Fact] + public async Task WriteResultsAsyncAllowsTrailingCommas() + { + await s_currentDirectoryLock.WaitAsync(); + string testRoot = CreateTempDirectory(); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + Directory.SetCurrentDirectory(testRoot); + Directory.CreateDirectory("docs"); + + await File.WriteAllTextAsync("docfx.json", """ + { + "build": { + "content": [ + { + "src": "docs", + "files": ["**/*.md",], + }, + ], + }, + } + """); + + using var writer = new StringWriter(); + bool result = await PathVerifier.WriteResultsAsync(writer); + + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + Directory.Delete(testRoot, recursive: true); + s_currentDirectoryLock.Release(); + } + } + private static string CreateTempDirectory() { string path = Path.Combine(Path.GetTempPath(), $"path-verifier-tests-{Guid.NewGuid():N}"); diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs index c0efb709..3269bcf0 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs @@ -1,5 +1,4 @@ using System.Net; -using System.Net.Sockets; using RedirectionVerifier; using Xunit; @@ -10,16 +9,14 @@ public class RedirectTargetVerifierTests [Fact] public async Task WriteResultsAsyncReturnsTrueForValidUrl() { - await using var server = new TestHttpServer(new Dictionary - { - ["/ok"] = HttpStatusCode.OK - }); - - string redirectionFilePath = await CreateRedirectionFileAsync($"{server.BaseUrl}/ok"); + string redirectionFilePath = await CreateRedirectionFileAsync("https://learn.microsoft.com/dotnet"); try { using var writer = new StringWriter(); - bool result = await RedirectTargetVerifier.WriteResultsAsync(writer, redirectionFilePath); + bool result = await RedirectTargetVerifier.WriteResultsAsync( + writer, + redirectionFilePath, + _ => Task.FromResult(HttpStatusCode.OK)); Assert.True(result); Assert.Equal(string.Empty, writer.ToString()); @@ -51,19 +48,45 @@ public async Task WriteResultsAsyncReturnsFalseForInvalidUrl() [Fact] public async Task WriteResultsAsyncReturnsFalseFor404Url() { - await using var server = new TestHttpServer(new Dictionary + string redirectionFilePath = await CreateRedirectionFileAsync("https://learn.microsoft.com/missing"); + try + { + using var writer = new StringWriter(); + bool result = await RedirectTargetVerifier.WriteResultsAsync( + writer, + redirectionFilePath, + _ => Task.FromResult(HttpStatusCode.NotFound)); + + Assert.False(result); + Assert.Contains("returns 404", writer.ToString(), StringComparison.Ordinal); + } + finally { - ["/missing"] = HttpStatusCode.NotFound - }); + File.Delete(redirectionFilePath); + } + } + + [Fact] + public async Task WriteResultsAsyncReturnsFalseForLocalAddressTarget() + { + string redirectionFilePath = await CreateRedirectionFileAsync("http://127.0.0.1/internal"); + bool statusProviderCalled = false; - string redirectionFilePath = await CreateRedirectionFileAsync($"{server.BaseUrl}/missing"); try { using var writer = new StringWriter(); - bool result = await RedirectTargetVerifier.WriteResultsAsync(writer, redirectionFilePath); + bool result = await RedirectTargetVerifier.WriteResultsAsync( + writer, + redirectionFilePath, + _ => + { + statusProviderCalled = true; + return Task.FromResult(HttpStatusCode.OK); + }); Assert.False(result); - Assert.Contains("returns 404", writer.ToString(), StringComparison.Ordinal); + Assert.Contains("Disallowed 'redirect_url' target", writer.ToString(), StringComparison.Ordinal); + Assert.False(statusProviderCalled); } finally { @@ -89,69 +112,4 @@ private static async Task CreateRedirectionFileAsync(string redirectUrl) return filePath; } - private sealed class TestHttpServer : IAsyncDisposable - { - private readonly HttpListener _listener; - private readonly Task _listenerTask; - private readonly Dictionary _responses; - - public TestHttpServer(Dictionary responses) - { - _responses = responses; - int port = GetFreePort(); - BaseUrl = $"http://127.0.0.1:{port}"; - - _listener = new HttpListener(); - _listener.Prefixes.Add($"{BaseUrl}/"); - _listener.Start(); - - _listenerTask = Task.Run(HandleRequestsAsync); - } - - public string BaseUrl { get; } - - private async Task HandleRequestsAsync() - { - while (_listener.IsListening) - { - HttpListenerContext? context; - try - { - context = await _listener.GetContextAsync(); - } - catch (HttpListenerException) - { - break; - } - catch (ObjectDisposedException) - { - break; - } - - string path = context.Request.Url?.AbsolutePath ?? "/"; - HttpStatusCode statusCode = _responses.TryGetValue(path, out HttpStatusCode configured) - ? configured - : HttpStatusCode.OK; - - context.Response.StatusCode = (int)statusCode; - context.Response.ContentLength64 = 0; - context.Response.Close(); - } - } - - public async ValueTask DisposeAsync() - { - _listener.Stop(); - _listener.Close(); - await _listenerTask; - } - - private static int GetFreePort() - { - using var tcpListener = new TcpListener(IPAddress.Loopback, 0); - tcpListener.Start(); - int port = ((IPEndPoint)tcpListener.LocalEndpoint).Port; - return port; - } - } } From 9b098fff6dc4509783404a9898ce3fada2979bd9 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:28:44 -0700 Subject: [PATCH 7/8] only verify fileMetadata paths --- .../src/DocfxVerifier/PathVerifier.cs | 168 +----------- .../GitHub.UnitTests/PathVerifierTests.cs | 259 +++--------------- 2 files changed, 49 insertions(+), 378 deletions(-) diff --git a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs index ee21d516..7d0ace4d 100644 --- a/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs +++ b/actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs @@ -3,7 +3,8 @@ namespace DocfxVerifier { /// - /// Validates file path entries declared in a docfx.json file. + /// Validates file path entries declared + /// under build.fileMetadata in a docfx.json file. /// public static class PathVerifier { @@ -46,7 +47,7 @@ public static async Task WriteResultsAsync(TextWriter writer, string? conf string configurationPathForLog = configurationPath.Replace('\\', '/'); var errors = new List(); - ValidateConfiguration(json.RootElement, repositoryRoot, configurationDirectory, errors); + ValidateFileMetadataPaths(json.RootElement, repositoryRoot, configurationDirectory, errors); foreach (string error in errors) { @@ -56,11 +57,11 @@ public static async Task WriteResultsAsync(TextWriter writer, string? conf return errors.Count == 0; } - private static void ValidateConfiguration( - JsonElement element, - string repositoryRoot, - string configurationDirectory, - List errors) + private static void ValidateFileMetadataPaths( + JsonElement element, + string repositoryRoot, + string configurationDirectory, + List errors) { if (element.ValueKind != JsonValueKind.Object) { @@ -70,32 +71,17 @@ private static void ValidateConfiguration( if (element.TryGetProperty("build", out JsonElement buildSection) && buildSection.ValueKind == JsonValueKind.Object) { - ValidateBuildSection(buildSection, "$.build", repositoryRoot, configurationDirectory, errors); - } - - if (element.TryGetProperty("metadata", out JsonElement metadataSection)) - { - ValidateFileMappingArray(metadataSection, "$.metadata", repositoryRoot, configurationDirectory, configurationDirectory, errors); + ValidateBuildFileMetadataSection(buildSection, "$.build", repositoryRoot, configurationDirectory, errors); } } - private static void ValidateBuildSection( + private static void ValidateBuildFileMetadataSection( JsonElement buildSection, string jsonPath, string repositoryRoot, string configurationDirectory, List errors) { - ValidateStringProperty(buildSection, "dest", $"{jsonPath}.dest", repositoryRoot, configurationDirectory, errors); - ValidateStringArrayProperty(buildSection, "template", $"{jsonPath}.template", repositoryRoot, configurationDirectory, errors); - ValidateStringArrayProperty(buildSection, "xref", $"{jsonPath}.xref", repositoryRoot, configurationDirectory, errors); - ValidateStringArrayProperty(buildSection, "globalMetadataFiles", $"{jsonPath}.globalMetadataFiles", repositoryRoot, configurationDirectory, errors); - ValidateStringArrayProperty(buildSection, "fileMetadataFiles", $"{jsonPath}.fileMetadataFiles", repositoryRoot, configurationDirectory, errors); - - ValidateFileMappingArrayProperty(buildSection, "content", $"{jsonPath}.content", repositoryRoot, configurationDirectory, errors); - ValidateFileMappingArrayProperty(buildSection, "resource", $"{jsonPath}.resource", repositoryRoot, configurationDirectory, errors); - ValidateFileMappingArrayProperty(buildSection, "overwrite", $"{jsonPath}.overwrite", repositoryRoot, configurationDirectory, errors); - if (buildSection.TryGetProperty("fileMetadata", out JsonElement fileMetadata) && fileMetadata.ValueKind == JsonValueKind.Object) { @@ -119,143 +105,11 @@ private static void ValidateBuildSection( } } - private static void ValidateStringProperty( - JsonElement parent, - string propertyName, - string jsonPath, - string repositoryRoot, - string resolutionBaseDirectory, - List errors) - { - if (parent.TryGetProperty(propertyName, out JsonElement property) - && property.ValueKind == JsonValueKind.String) - { - ValidatePath(property.GetString(), jsonPath, repositoryRoot, resolutionBaseDirectory, errors); - } - } - - private static void ValidateStringArrayProperty( - JsonElement parent, - string propertyName, - string jsonPath, - string repositoryRoot, - string resolutionBaseDirectory, - List errors) - { - if (parent.TryGetProperty(propertyName, out JsonElement property)) - { - ValidateStringArray(property, jsonPath, repositoryRoot, resolutionBaseDirectory, errors); - } - } - - private static void ValidateFileMappingArrayProperty( - JsonElement parent, - string propertyName, - string jsonPath, - string repositoryRoot, - string configurationDirectory, - List errors) - { - if (parent.TryGetProperty(propertyName, out JsonElement property)) - { - ValidateFileMappingArray(property, jsonPath, repositoryRoot, configurationDirectory, configurationDirectory, errors); - } - } - - private static void ValidateFileMappingArray( - JsonElement mappings, - string jsonPath, - string repositoryRoot, - string configurationDirectory, - string resolutionBaseDirectory, - List errors) - { - if (mappings.ValueKind != JsonValueKind.Array) - { - return; - } - - int index = 0; - foreach (JsonElement mapping in mappings.EnumerateArray()) - { - string mappingPath = $"{jsonPath}[{index}]"; - if (mapping.ValueKind == JsonValueKind.String) - { - ValidatePath(mapping.GetString(), mappingPath, repositoryRoot, resolutionBaseDirectory, errors); - } - else if (mapping.ValueKind == JsonValueKind.Object) - { - ValidateFileMappingObject(mapping, mappingPath, repositoryRoot, configurationDirectory, errors); - } - - index++; - } - } - - private static void ValidateFileMappingObject( - JsonElement mapping, - string jsonPath, - string repositoryRoot, - string configurationDirectory, - List errors) - { - string mappingSourceDirectory = configurationDirectory; - - if (mapping.TryGetProperty("src", out JsonElement src) - && src.ValueKind == JsonValueKind.String) - { - string? srcPath = src.GetString(); - ValidatePath(srcPath, $"{jsonPath}.src", repositoryRoot, configurationDirectory, errors); - - string? effectiveSourceDirectory = TryResolvePathWithinRepository(srcPath, repositoryRoot, configurationDirectory); - if (effectiveSourceDirectory is not null) - { - mappingSourceDirectory = effectiveSourceDirectory; - } - } - - ValidateStringProperty(mapping, "dest", $"{jsonPath}.dest", repositoryRoot, configurationDirectory, errors); - - if (mapping.TryGetProperty("files", out JsonElement files)) - { - ValidateStringArray(files, $"{jsonPath}.files", repositoryRoot, mappingSourceDirectory, errors); - } - - if (mapping.TryGetProperty("exclude", out JsonElement exclude)) - { - ValidateStringArray(exclude, $"{jsonPath}.exclude", repositoryRoot, mappingSourceDirectory, errors); - } - } - - private static void ValidateStringArray( - JsonElement values, - string jsonPath, - string repositoryRoot, - string resolutionBaseDirectory, - List errors) - { - if (values.ValueKind != JsonValueKind.Array) - { - return; - } - - int index = 0; - foreach (JsonElement item in values.EnumerateArray()) - { - if (item.ValueKind == JsonValueKind.String) - { - ValidatePath(item.GetString(), $"{jsonPath}[{index}]", repositoryRoot, resolutionBaseDirectory, errors); - } - - index++; - } - } - private static void ValidatePath( string? path, string jsonPath, string repositoryRoot, - string resolutionBaseDirectory, + string resolutionBaseDirectory, List errors) { if (string.IsNullOrWhiteSpace(path) || path is ".") diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs index 0574d0cf..3baaa173 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs @@ -8,7 +8,7 @@ public class PathVerifierTests private static readonly SemaphoreSlim s_currentDirectoryLock = new(1, 1); [Fact] - public async Task WriteResultsAsyncReturnsTrueForValidPaths() + public async Task WriteResultsAsyncReturnsTrueForValidFileMetadataPaths() { await s_currentDirectoryLock.WaitAsync(); string testRoot = CreateTempDirectory(); @@ -17,20 +17,15 @@ public async Task WriteResultsAsyncReturnsTrueForValidPaths() try { Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory("docs"); - Directory.CreateDirectory(Path.Combine("templates", "default")); + Directory.CreateDirectory(Path.Combine("docs", "valid")); await File.WriteAllTextAsync("docfx.json", """ { "build": { - "content": [ - { - "files": ["**/*.md", "**/*.yml"], - "src": "docs", - "exclude": ["**/includes/**"] + "fileMetadata": { + "ms.author": { + "docs/valid/**/**.{md,yml}": "someone" } - ], - "template": ["templates/default"], - "xref": ["https://learn.microsoft.com/dotnet/.xrefmap.json"] + } } } """); @@ -50,7 +45,7 @@ await File.WriteAllTextAsync("docfx.json", """ } [Fact] - public async Task WriteResultsAsyncValidatesFileMetadataPaths() + public async Task WriteResultsAsyncReturnsFalseForInvalidFileMetadataPaths() { await s_currentDirectoryLock.WaitAsync(); string testRoot = CreateTempDirectory(); @@ -59,20 +54,11 @@ public async Task WriteResultsAsyncValidatesFileMetadataPaths() try { Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory(Path.Combine("docs", "valid")); - await File.WriteAllTextAsync("docfx.json", """ { "build": { - "content": [ - { - "files": ["**/*.md"], - "src": "docs" - } - ], "fileMetadata": { "ms.author": { - "docs/valid/**/**.{md,yml}": "someone", "missing/path/**/**.{md,yml}": "someone" } } @@ -86,7 +72,6 @@ await File.WriteAllTextAsync("docfx.json", """ Assert.False(result); Assert.Contains("Path 'missing/path/**/**.{md,yml}' is invalid", output, StringComparison.Ordinal); - Assert.DoesNotContain("Path 'docs/valid/**/**.{md,yml}' is invalid", output, StringComparison.Ordinal); } finally { @@ -97,7 +82,7 @@ await File.WriteAllTextAsync("docfx.json", """ } [Fact] - public async Task WriteResultsAsyncReturnsFalseForInvalidPaths() + public async Task WriteResultsAsyncIgnoresNonFileMetadataPathEntries() { await s_currentDirectoryLock.WaitAsync(); string testRoot = CreateTempDirectory(); @@ -114,7 +99,16 @@ await File.WriteAllTextAsync("docfx.json", """ "files": ["**/*.md"], "src": "missing-folder" } - ] + ], + "template": ["missing-template"], + "fileMetadata": { + "ms.author": { + "**/*.md": "someone" + } + } + }, + "globalMetadata": { + "src": "not-a-docfx-path" } } """); @@ -122,8 +116,8 @@ await File.WriteAllTextAsync("docfx.json", """ using var writer = new StringWriter(); bool result = await PathVerifier.WriteResultsAsync(writer); - Assert.False(result); - Assert.Contains("Path 'missing-folder' is invalid", writer.ToString(), StringComparison.Ordinal); + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); } finally { @@ -143,19 +137,15 @@ public async Task WriteResultsAsyncFindsDocfxInSubdirectory() try { Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory("docs"); - Directory.CreateDirectory("docs/content"); - Directory.CreateDirectory(Path.Combine("docs", "templates", "default")); + Directory.CreateDirectory(Path.Combine("docs", "content", "guides")); await File.WriteAllTextAsync(Path.Combine("docs", "docfx.json"), """ { "build": { - "content": [ - { - "files": ["**/*.md"], - "src": "content" + "fileMetadata": { + "ms.topic": { + "content/**": "conceptual" } - ], - "template": ["templates/default"] + } } } """); @@ -185,31 +175,28 @@ public async Task WriteResultsAsyncUsesSpecifiedDocfxPath() { Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory("valid-docs"); - await File.WriteAllTextAsync("docfx.json", """ { "build": { - "content": [ - { - "files": ["**/*.md"], - "src": "missing-root-folder" + "fileMetadata": { + "ms.author": { + "missing-root-folder/**": "someone" } - ] + } } } """); + Directory.CreateDirectory(Path.Combine("valid-docs", "valid")); string modifiedDocfxPath = Path.Combine("valid-docs", "docfx.json"); await File.WriteAllTextAsync(modifiedDocfxPath, """ { "build": { - "content": [ - { - "files": ["**/*.md"], - "src": "." + "fileMetadata": { + "ms.author": { + "valid/**": "someone" } - ] + } } } """); @@ -228,175 +215,6 @@ await File.WriteAllTextAsync(modifiedDocfxPath, """ } } - [Fact] - public async Task WriteResultsAsyncValidatesFilesRelativeToMappingSrc() - { - await s_currentDirectoryLock.WaitAsync(); - string testRoot = CreateTempDirectory(); - string originalDirectory = Directory.GetCurrentDirectory(); - - try - { - Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory(Path.Combine("docs", "content", "guides")); - await File.WriteAllTextAsync(Path.Combine("docs", "content", "guides", "a.md"), "# title"); - - string configurationPath = Path.Combine("docs", "docfx.json"); - await File.WriteAllTextAsync(configurationPath, """ - { - "build": { - "content": [ - { - "src": "content", - "files": ["guides/a.md"] - } - ] - } - } - """); - - using var writer = new StringWriter(); - bool result = await PathVerifier.WriteResultsAsync(writer, configurationPath); - - Assert.True(result); - Assert.Equal(string.Empty, writer.ToString()); - } - finally - { - Directory.SetCurrentDirectory(originalDirectory); - Directory.Delete(testRoot, recursive: true); - s_currentDirectoryLock.Release(); - } - } - - [Fact] - public async Task WriteResultsAsyncAllowsParentRelativePathWhenInsideRepository() - { - await s_currentDirectoryLock.WaitAsync(); - string testRoot = CreateTempDirectory(); - string originalDirectory = Directory.GetCurrentDirectory(); - - try - { - Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory(Path.Combine("shared", "templates")); - Directory.CreateDirectory("docs"); - - string configurationPath = Path.Combine("docs", "docfx.json"); - await File.WriteAllTextAsync(configurationPath, """ - { - "build": { - "template": ["../shared/templates"] - } - } - """); - - using var writer = new StringWriter(); - bool result = await PathVerifier.WriteResultsAsync(writer, configurationPath); - - Assert.True(result); - Assert.Equal(string.Empty, writer.ToString()); - } - finally - { - Directory.SetCurrentDirectory(originalDirectory); - Directory.Delete(testRoot, recursive: true); - s_currentDirectoryLock.Release(); - } - } - - [Fact] - public async Task WriteResultsAsyncSupportsFileMappingShorthandAndObjectForms() - { - await s_currentDirectoryLock.WaitAsync(); - string testRoot = CreateTempDirectory(); - string originalDirectory = Directory.GetCurrentDirectory(); - - try - { - Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory(Path.Combine("docs", "articles")); - Directory.CreateDirectory(Path.Combine("docs", "assets", "images")); - Directory.CreateDirectory(Path.Combine("docs", "content", "overwrite")); - await File.WriteAllTextAsync(Path.Combine("docs", "assets", "images", "logo.png"), "binary"); - - string configurationPath = Path.Combine("docs", "docfx.json"); - await File.WriteAllTextAsync(configurationPath, """ - { - "build": { - "content": ["articles/**/*.md"], - "resource": [ - { - "src": "assets", - "files": ["images/logo.png"] - } - ], - "overwrite": [ - { - "src": "content", - "files": ["overwrite/**/*.md"] - } - ] - } - } - """); - - using var writer = new StringWriter(); - bool result = await PathVerifier.WriteResultsAsync(writer, configurationPath); - - Assert.True(result); - Assert.Equal(string.Empty, writer.ToString()); - } - finally - { - Directory.SetCurrentDirectory(originalDirectory); - Directory.Delete(testRoot, recursive: true); - s_currentDirectoryLock.Release(); - } - } - - [Fact] - public async Task WriteResultsAsyncIgnoresArbitraryMetadataPropertyNames() - { - await s_currentDirectoryLock.WaitAsync(); - string testRoot = CreateTempDirectory(); - string originalDirectory = Directory.GetCurrentDirectory(); - - try - { - Directory.SetCurrentDirectory(testRoot); - Directory.CreateDirectory("docs"); - - await File.WriteAllTextAsync("docfx.json", """ - { - "build": { - "content": [ - { - "src": "docs", - "files": ["**/*.md"] - } - ] - }, - "globalMetadata": { - "src": "not-a-docfx-path" - } - } - """); - - using var writer = new StringWriter(); - bool result = await PathVerifier.WriteResultsAsync(writer); - - Assert.True(result); - Assert.Equal(string.Empty, writer.ToString()); - } - finally - { - Directory.SetCurrentDirectory(originalDirectory); - Directory.Delete(testRoot, recursive: true); - s_currentDirectoryLock.Release(); - } - } - [Fact] public async Task WriteResultsAsyncAllowsTrailingCommas() { @@ -412,12 +230,11 @@ public async Task WriteResultsAsyncAllowsTrailingCommas() await File.WriteAllTextAsync("docfx.json", """ { "build": { - "content": [ - { - "src": "docs", - "files": ["**/*.md",], + "fileMetadata": { + "ms.author": { + "docs/**": "someone", }, - ], + }, }, } """); From f898694794ad747befb5d50468537e33dde4af96 Mon Sep 17 00:00:00 2001 From: Genevieve Warren <24882762+gewarren@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:31:14 -0700 Subject: [PATCH 8/8] simplify verification logic --- .../RedirectTargetVerifier.cs | 75 ++----------------- .../RedirectTargetVerifierTests.cs | 38 +++++----- 2 files changed, 27 insertions(+), 86 deletions(-) diff --git a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs index 13eb3a5d..5d542787 100644 --- a/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs +++ b/actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs @@ -1,6 +1,5 @@ using System.Collections.Immutable; using System.Net; -using System.Net.Sockets; namespace RedirectionVerifier; @@ -53,9 +52,12 @@ internal static async Task WriteResultsAsync( continue; } - string redirectTarget = redirectUrl.StartsWith('/') - ? $"{LearnMicrosoftCom}{redirectUrl}" - : redirectUrl; + if (redirectUrl[0] != '/') + { + continue; + } + + string redirectTarget = $"{LearnMicrosoftCom}{redirectUrl}"; bool hasValidUri = Uri.TryCreate(redirectTarget, UriKind.Absolute, out Uri? uri) && uri is not null @@ -68,13 +70,6 @@ internal static async Task WriteResultsAsync( continue; } - if (!await IsPublicHttpTargetAsync(uri!)) - { - await writer.WriteLineAsync($"::error file={redirectionFilePath}::Disallowed 'redirect_url' target at index {i}: '{redirectUrl}'."); - isValid = false; - continue; - } - HttpStatusCode? statusCode = await statusCodeProvider(uri!); if (statusCode is null) { @@ -93,64 +88,6 @@ internal static async Task WriteResultsAsync( return isValid; } - private static async Task IsPublicHttpTargetAsync(Uri uri) - { - if (uri.IsLoopback) - { - return false; - } - - if (IPAddress.TryParse(uri.Host, out IPAddress? parsedAddress)) - { - return !IsPrivateOrLocalAddress(parsedAddress); - } - - try - { - IPAddress[] addresses = await Dns.GetHostAddressesAsync(uri.Host); - return addresses.All(address => !IsPrivateOrLocalAddress(address)); - } - catch (SocketException) - { - return true; - } - } - - private static bool IsPrivateOrLocalAddress(IPAddress address) - { - if (IPAddress.IsLoopback(address)) - { - return true; - } - - if (address.AddressFamily == AddressFamily.InterNetwork) - { - byte[] bytes = address.GetAddressBytes(); - return bytes[0] switch - { - 10 => true, - 127 => true, - 169 when bytes[1] == 254 => true, - 172 when bytes[1] >= 16 && bytes[1] <= 31 => true, - 192 when bytes[1] == 168 => true, - _ => false - }; - } - - if (address.AddressFamily == AddressFamily.InterNetworkV6) - { - if (address.IsIPv6LinkLocal || address.IsIPv6SiteLocal) - { - return true; - } - - byte[] bytes = address.GetAddressBytes(); - return (bytes[0] & 0xFE) == 0xFC; - } - - return false; - } - private static async Task GetStatusCodeAsync(Uri uri) { try diff --git a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs index 3269bcf0..99a611ef 100644 --- a/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs +++ b/actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs @@ -7,9 +7,9 @@ namespace GitHub.UnitTests; public class RedirectTargetVerifierTests { [Fact] - public async Task WriteResultsAsyncReturnsTrueForValidUrl() + public async Task WriteResultsAsyncReturnsTrueForValidLearnUrlPath() { - string redirectionFilePath = await CreateRedirectionFileAsync("https://learn.microsoft.com/dotnet"); + string redirectionFilePath = await CreateRedirectionFileAsync("/dotnet"); try { using var writer = new StringWriter(); @@ -28,16 +28,26 @@ public async Task WriteResultsAsyncReturnsTrueForValidUrl() } [Fact] - public async Task WriteResultsAsyncReturnsFalseForInvalidUrl() + public async Task WriteResultsAsyncSkipsNonLearnUrlTargets() { string redirectionFilePath = await CreateRedirectionFileAsync("not-a-valid-url"); + bool statusProviderCalled = false; + try { using var writer = new StringWriter(); - bool result = await RedirectTargetVerifier.WriteResultsAsync(writer, redirectionFilePath); + bool result = await RedirectTargetVerifier.WriteResultsAsync( + writer, + redirectionFilePath, + _ => + { + statusProviderCalled = true; + return Task.FromResult(HttpStatusCode.OK); + }); - Assert.False(result); - Assert.Contains("Invalid 'redirect_url'", writer.ToString(), StringComparison.Ordinal); + Assert.True(result); + Assert.Equal(string.Empty, writer.ToString()); + Assert.False(statusProviderCalled); } finally { @@ -48,7 +58,7 @@ public async Task WriteResultsAsyncReturnsFalseForInvalidUrl() [Fact] public async Task WriteResultsAsyncReturnsFalseFor404Url() { - string redirectionFilePath = await CreateRedirectionFileAsync("https://learn.microsoft.com/missing"); + string redirectionFilePath = await CreateRedirectionFileAsync("/missing"); try { using var writer = new StringWriter(); @@ -67,10 +77,9 @@ public async Task WriteResultsAsyncReturnsFalseFor404Url() } [Fact] - public async Task WriteResultsAsyncReturnsFalseForLocalAddressTarget() + public async Task WriteResultsAsyncReturnsFalseWhenLearnUrlCannotBeVerified() { - string redirectionFilePath = await CreateRedirectionFileAsync("http://127.0.0.1/internal"); - bool statusProviderCalled = false; + string redirectionFilePath = await CreateRedirectionFileAsync("/dotnet"); try { @@ -78,15 +87,10 @@ public async Task WriteResultsAsyncReturnsFalseForLocalAddressTarget() bool result = await RedirectTargetVerifier.WriteResultsAsync( writer, redirectionFilePath, - _ => - { - statusProviderCalled = true; - return Task.FromResult(HttpStatusCode.OK); - }); + _ => Task.FromResult(null)); Assert.False(result); - Assert.Contains("Disallowed 'redirect_url' target", writer.ToString(), StringComparison.Ordinal); - Assert.False(statusProviderCalled); + Assert.Contains("Unable to verify 'redirect_url'", writer.ToString(), StringComparison.Ordinal); } finally {