diff --git a/src/backend/PkgDiff.Application/Interfaces/IGitHubScraperService.cs b/src/backend/PkgDiff.Application/Interfaces/IGitHubScraperService.cs index db87464..6a178df 100644 --- a/src/backend/PkgDiff.Application/Interfaces/IGitHubScraperService.cs +++ b/src/backend/PkgDiff.Application/Interfaces/IGitHubScraperService.cs @@ -11,4 +11,7 @@ public interface IGitHubScraperService // The package name tells a monorepo's per-package tags apart Task GetReleaseNotesForVersionAsync(string githubRepoUrl, string version, string? packageId) => GetReleaseNotesForVersionAsync(githubRepoUrl, version); + + // False for a private, missing or malformed repository; null when it could not be checked + Task IsPublicRepositoryAsync(string githubRepoUrl) => Task.FromResult(null); } diff --git a/src/backend/PkgDiff.Application/Interfaces/IPackageCacheService.cs b/src/backend/PkgDiff.Application/Interfaces/IPackageCacheService.cs index c828f60..aa74549 100644 --- a/src/backend/PkgDiff.Application/Interfaces/IPackageCacheService.cs +++ b/src/backend/PkgDiff.Application/Interfaces/IPackageCacheService.cs @@ -15,4 +15,7 @@ public interface IPackageCacheService Task SaveReleaseNoteAsync(CachedReleaseNote releaseNote); Task GetOrFetchMetadataAsync(string registry, string packageId, bool bypassCache = false); Task ResolveRegistryIdAsync(string registryValue); + + // Null for a repository that is private, missing or not a repository + Task UsableRepositoryUrlAsync(string? repositoryUrl); } diff --git a/src/backend/PkgDiff.Application/Services/PackageAnalysisService.cs b/src/backend/PkgDiff.Application/Services/PackageAnalysisService.cs index a8454d1..c35fd6a 100644 --- a/src/backend/PkgDiff.Application/Services/PackageAnalysisService.cs +++ b/src/backend/PkgDiff.Application/Services/PackageAnalysisService.cs @@ -425,9 +425,10 @@ private async Task ScrapeVersionReleaseNotesAsync( var releaseNotes = StoredMarkdown.Normalize(result.ReleaseNotes); PkgDiffTelemetry.RecordScrape(activity, registry.Id, result.Source, !string.IsNullOrWhiteSpace(releaseNotes)); - if (result.DiscoveredGitHubUrl != null) + if (result.DiscoveredGitHubUrl != null + && await _cacheService.UsableRepositoryUrlAsync(result.DiscoveredGitHubUrl) is { } discoveredUrl) { - cachedPackage.GitHubRepositoryUrl = result.DiscoveredGitHubUrl; + cachedPackage.GitHubRepositoryUrl = discoveredUrl; cachedPackage.ReleaseNotesSource = "GitHub"; await _cacheService.UpdateCachedPackageAsync(cachedPackage); } diff --git a/src/backend/PkgDiff.Application/Services/PackageCacheService.cs b/src/backend/PkgDiff.Application/Services/PackageCacheService.cs index 8edadc8..5ae0283 100644 --- a/src/backend/PkgDiff.Application/Services/PackageCacheService.cs +++ b/src/backend/PkgDiff.Application/Services/PackageCacheService.cs @@ -18,6 +18,7 @@ public class PackageCacheService : IPackageCacheService private readonly ICachedLookups _lookups; private readonly IJobScheduler _jobs; private readonly ILogger _logger; + private readonly IGitHubScraperService? _gitHub; public PackageCacheService( IUnitOfWork unitOfWork, @@ -26,7 +27,8 @@ public PackageCacheService( ILiveUpdateService liveUpdates, ICachedLookups lookups, IJobScheduler jobs, - ILogger logger) + ILogger logger, + IGitHubScraperService? gitHub = null) { _unitOfWork = unitOfWork; _registryFactory = registryFactory; @@ -35,6 +37,7 @@ public PackageCacheService( _lookups = lookups; _jobs = jobs; _logger = logger; + _gitHub = gitHub; } public async Task GetCachedPackageAsync(string packageId, string registry) @@ -180,6 +183,14 @@ public Task ResolveRegistryIdAsync(string registryValue) return _lookups.GetRegistryIdAsync(registryValue); } + public async Task UsableRepositoryUrlAsync(string? repositoryUrl) + { + if (string.IsNullOrWhiteSpace(repositoryUrl) || _gitHub == null) + return repositoryUrl; + + return await _gitHub.IsPublicRepositoryAsync(repositoryUrl) == false ? null : repositoryUrl; + } + private async Task BuildMetadataFromCache(CachedPackage cached, string registryLower, IRegistryClient registry) { var cachedNotes = await _unitOfWork.CachedReleaseNotes @@ -253,6 +264,10 @@ private async Task SaveFreshMetadataToCache( { var registryId = await ResolveRegistryIdAsync(registry); + // Checked again on every refresh, so a repository that turns public or private is picked up + var namedRepository = metadata.GitHubRepositoryUrl; + metadata.GitHubRepositoryUrl = await UsableRepositoryUrlAsync(namedRepository); + var repositoryChanged = false; if (cached == null) { @@ -282,7 +297,9 @@ private async Task SaveFreshMetadataToCache( repositoryChanged = metadata.GitHubRepositoryUrl != null && !string.Equals(metadata.GitHubRepositoryUrl, cached.GitHubRepositoryUrl, StringComparison.OrdinalIgnoreCase); // A repository found while scraping release notes is kept when the registry names none - cached.GitHubRepositoryUrl = metadata.GitHubRepositoryUrl ?? cached.GitHubRepositoryUrl; + cached.GitHubRepositoryUrl = namedRepository != null + ? metadata.GitHubRepositoryUrl + : await UsableRepositoryUrlAsync(cached.GitHubRepositoryUrl); cached.AllVersionsJson = JsonSerializer.Serialize(metadata.Versions.Select(v => v.Version)); cached.CachedAt = DateTime.UtcNow; cached.ExpiresAt = DateTime.UtcNow.AddDays(14); diff --git a/src/backend/PkgDiff.Infrastructure/Services/GitHubScrapeCache.cs b/src/backend/PkgDiff.Infrastructure/Services/GitHubScrapeCache.cs index 143aab9..2ec46cc 100644 --- a/src/backend/PkgDiff.Infrastructure/Services/GitHubScrapeCache.cs +++ b/src/backend/PkgDiff.Infrastructure/Services/GitHubScrapeCache.cs @@ -17,8 +17,25 @@ public sealed class GitHubScrapeCache : IDisposable // A repository that had no changelog file may get one, so this knowledge is not kept for good private static readonly TimeSpan ChangelogLifetime = TimeSpan.FromHours(12); + // A repository can be made public or private at any time + private static readonly TimeSpan VisibilityLifetime = TimeSpan.FromHours(6); + private readonly MemoryCache _cache = new(new MemoryCacheOptions { SizeLimit = MaxEntries }); + public bool TryGetVisibility(string repoKey, out bool isPublic) + { + return _cache.TryGetValue("visibility:" + repoKey, out isPublic); + } + + public void SetVisibility(string repoKey, bool isPublic) + { + _cache.Set("visibility:" + repoKey, isPublic, new MemoryCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = VisibilityLifetime, + Size = 1 + }); + } + public bool TryGetReleases(string repositoryUrl, [NotNullWhen(true)] out Dictionary? releases) { return _cache.TryGetValue(ReleasesKey(repositoryUrl), out releases) && releases != null; diff --git a/src/backend/PkgDiff.Infrastructure/Services/GitHubScraperService.cs b/src/backend/PkgDiff.Infrastructure/Services/GitHubScraperService.cs index b572d4f..e9f09be 100644 --- a/src/backend/PkgDiff.Infrastructure/Services/GitHubScraperService.cs +++ b/src/backend/PkgDiff.Infrastructure/Services/GitHubScraperService.cs @@ -21,6 +21,7 @@ public partial class GitHubScraperService : IGitHubScraperService private readonly HttpClient _httpClient; private readonly bool _enableAuthentication; private readonly bool _useGitHubApi; + private readonly bool _sendsToken; private readonly GitHubScrapeCache _cache; private readonly GitHubChangelogReader _changelog; private readonly GitHubMilestoneReader _milestones; @@ -53,7 +54,8 @@ public GitHubScraperService( _httpClient.DefaultRequestHeaders.Add("User-Agent", userAgent); _httpClient.DefaultRequestHeaders.Add("Accept", "application/vnd.github+json"); - if (_enableAuthentication && !string.IsNullOrWhiteSpace(personalAccessToken)) + _sendsToken = _enableAuthentication && !string.IsNullOrWhiteSpace(personalAccessToken); + if (_sendsToken) { _httpClient.DefaultRequestHeaders.Add("Authorization", $"Bearer {personalAccessToken}"); } @@ -107,7 +109,10 @@ public async Task> GetAllReleasesAsync(string githubR return await _retryPolicy.ExecuteAsync(async () => { var cleanUrl = GitHubUrls.Clean(githubRepoUrl); - + + if (!await MayReadAsync(cleanUrl)) + return new Dictionary(); + if (_cache.TryGetReleases(cleanUrl, out var cachedReleases)) { _logger.LogDebug("[GitHubScraper] Using cached releases for {RepoUrl} ({Count} releases)", @@ -241,6 +246,9 @@ public async Task> GetAllReleasesAsync(string githubR { _logger.LogDebug("[GitHubScraper] Looking for release notes: repo={RepoUrl}, version={Version}", githubRepoUrl, version); + if (!await MayReadAsync(githubRepoUrl)) + return null; + string? notes = null; if (!_useGitHubApi || !_enableAuthentication) @@ -313,6 +321,81 @@ public async Task> GetAllReleasesAsync(string githubR }); } + public async Task IsPublicRepositoryAsync(string githubRepoUrl) + { + try + { + return await RepositoryVisibilityAsync(githubRepoUrl); + } + catch (GitHubRateLimitedException) + { + return null; + } + } + + // The token may see private repositories; their content must never be published + private async Task MayReadAsync(string githubRepoUrl) + { + var isPublic = await RepositoryVisibilityAsync(githubRepoUrl); + if (isPublic == true || (isPublic == null && !_sendsToken)) + return true; + + _logger.LogInformation("[GitHubScraper] Not reading {RepoUrl}: not a public repository ({Visibility})", + githubRepoUrl, isPublic == false ? "private or missing" : "unchecked"); + return false; + } + + private async Task RepositoryVisibilityAsync(string githubRepoUrl) + { + var (owner, repo) = GitHubUrls.Parse(githubRepoUrl, _logger); + if (string.IsNullOrEmpty(owner) || string.IsNullOrEmpty(repo)) + return false; + + // Without a token nothing private can be read + if (!_sendsToken) + return null; + + var repoKey = $"{owner}/{repo}".ToLowerInvariant(); + if (_cache.TryGetVisibility(repoKey, out var cached)) + return cached; + + var apiUrl = $"https://api.github.com/repos/{PackageUrlEncoder.EscapeSegment(owner)}/{PackageUrlEncoder.EscapeSegment(repo)}"; + try + { + await _rateLimitHandler.CanMakeRequestAsync(); + using var response = await _rateLimiter.ExecuteAsync("github-api", async () => await _httpClient.GetAsync(apiUrl)); + await _rateLimitHandler.UpdateFromHeadersAsync(response); + GitHubRateLimitResponse.ThrowIfLimited(response); + + bool? isPublic = null; + if (response.StatusCode == System.Net.HttpStatusCode.NotFound) + { + isPublic = false; + } + else if (response.IsSuccessStatusCode) + { + using var document = JsonDocument.Parse(await response.Content.ReadAsStringAsync()); + isPublic = document.RootElement.TryGetProperty("private", out var isPrivate) + && isPrivate.ValueKind == JsonValueKind.False; + } + + if (isPublic.HasValue) + _cache.SetVisibility(repoKey, isPublic.Value); + else + _logger.LogWarning("[GitHubScraper] API returned {StatusCode} for {Url}", response.StatusCode, apiUrl); + return isPublic; + } + catch (GitHubRateLimitedException) + { + throw; + } + catch (Exception ex) + { + _logger.LogWarning(ex, "[GitHubScraper] Failed to read {Url}", apiUrl); + return null; + } + } + // Tag spellings of monorepos and package-named tags, most specific first private static IEnumerable TagCandidates(string? packageId, string version) { diff --git a/src/backend/PkgDiff.Infrastructure/Services/PubDevService.cs b/src/backend/PkgDiff.Infrastructure/Services/PubDevService.cs index 03be303..95cd238 100644 --- a/src/backend/PkgDiff.Infrastructure/Services/PubDevService.cs +++ b/src/backend/PkgDiff.Infrastructure/Services/PubDevService.cs @@ -25,58 +25,75 @@ public async Task GetReleaseNotesAsync(string packageId, string version) { try { - var url = $"{_baseUrl}/{PackageUrlEncoder.EscapeSegment(packageId)}/changelog"; - var html = await _httpClient.GetStringAsync(url); - var doc = new HtmlDocument(); - doc.LoadHtml(html); + var packageUrl = $"{_baseUrl}/{PackageUrlEncoder.EscapeSegment(packageId)}"; + // The package's changelog page shows only the latest upload; each version's page keeps the file it shipped with + return await ReadChangelogEntryAsync($"{packageUrl}/versions/{PackageUrlEncoder.EscapeSegment(version)}/changelog", version) + ?? await ReadChangelogEntryAsync($"{packageUrl}/changelog", version) + ?? string.Empty; + } + catch (Exception ex) + { + _logger.LogError(ex, "Error fetching release notes from Pub.dev for package {PackageId} version {Version}", packageId, version); + return string.Empty; + } + } - var changelogNodes = doc.DocumentNode.SelectNodes("//div[@class='changelog-entry']"); - if (changelogNodes == null || !changelogNodes.Any()) - return string.Empty; + // Null when the page or its entry for the version is missing + private async Task ReadChangelogEntryAsync(string url, string version) + { + using var response = await _httpClient.GetAsync(url); + if (!response.IsSuccessStatusCode) + return null; - var versionId = version.Replace(".", ""); // pub.dev changelog anchors drop the dots + var doc = new HtmlDocument(); + doc.LoadHtml(await response.Content.ReadAsStringAsync()); - foreach (var node in changelogNodes) - { - var headerNode = node.SelectSingleNode(".//h2"); - if (headerNode != null && headerNode.InnerText.Trim().StartsWith(version)) - { - var contentNode = node.SelectSingleNode(".//div[@class='changelog-content']"); - if (contentNode == null) return string.Empty; + var changelogNodes = doc.DocumentNode.SelectNodes("//div[@class='changelog-entry']"); + if (changelogNodes == null) + return null; - var rawHtml = contentNode.InnerHtml; + foreach (var node in changelogNodes) + { + var headerNode = node.SelectSingleNode(".//h2"); + if (headerNode == null || !IsHeaderOf(headerNode.InnerText, version)) + continue; - rawHtml = Regex.Replace(rawHtml, "]*>", "\n", RegexOptions.IgnoreCase); - rawHtml = Regex.Replace(rawHtml, "

", "\n\n", RegexOptions.IgnoreCase); - rawHtml = Regex.Replace(rawHtml, "]*>", "\n- ", RegexOptions.IgnoreCase); - rawHtml = Regex.Replace(rawHtml, "]*>", "\n# ", RegexOptions.IgnoreCase); + var contentNode = node.SelectSingleNode(".//div[@class='changelog-content']"); + if (contentNode == null) return string.Empty; - rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline); - rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline); - rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline); - rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline); - rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "`$1`", RegexOptions.IgnoreCase | RegexOptions.Singleline); - rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "\n```\n$1\n```\n", RegexOptions.IgnoreCase | RegexOptions.Singleline); + var rawHtml = contentNode.InnerHtml; - rawHtml = Regex.Replace(rawHtml, "]+href=\"([^\"]+)\"[^>]*>(.*?)", "[$2]($1)", RegexOptions.IgnoreCase | RegexOptions.Singleline); + rawHtml = Regex.Replace(rawHtml, "]*>", "\n", RegexOptions.IgnoreCase); + rawHtml = Regex.Replace(rawHtml, "

", "\n\n", RegexOptions.IgnoreCase); + rawHtml = Regex.Replace(rawHtml, "]*>", "\n- ", RegexOptions.IgnoreCase); + rawHtml = Regex.Replace(rawHtml, "]*>", "\n# ", RegexOptions.IgnoreCase); - var tempDoc = new HtmlDocument(); - tempDoc.LoadHtml(rawHtml); - var plainText = tempDoc.DocumentNode.InnerText; + rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline); + rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "**$1**", RegexOptions.IgnoreCase | RegexOptions.Singleline); + rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline); + rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "*$1*", RegexOptions.IgnoreCase | RegexOptions.Singleline); + rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "`$1`", RegexOptions.IgnoreCase | RegexOptions.Singleline); + rawHtml = Regex.Replace(rawHtml, "]*>(.*?)", "\n```\n$1\n```\n", RegexOptions.IgnoreCase | RegexOptions.Singleline); - plainText = Regex.Replace(plainText, @"\n{3,}", "\n\n"); + rawHtml = Regex.Replace(rawHtml, "]+href=\"([^\"]+)\"[^>]*>(.*?)", "[$2]($1)", RegexOptions.IgnoreCase | RegexOptions.Singleline); - return plainText.Trim(); - } - } + var tempDoc = new HtmlDocument(); + tempDoc.LoadHtml(rawHtml); + var plainText = tempDoc.DocumentNode.InnerText; - return string.Empty; - } - catch (Exception ex) - { - _logger.LogError(ex, "Error fetching release notes from Pub.dev for package {PackageId} version {Version}", packageId, version); - return string.Empty; + plainText = Regex.Replace(plainText, @"\n{3,}", "\n\n"); + + return plainText.Trim(); } + + return null; + } + + private static bool IsHeaderOf(string headerText, string version) + { + var firstWord = headerText.Split((char[]?)null, 2, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault() ?? string.Empty; + return firstWord.Trim('[', ']', '(', ')', ':', ',').TrimStart('v', 'V') + .Equals(version, StringComparison.OrdinalIgnoreCase); } public async Task GetGitHubRepositoryUrlAsync(string packageId) diff --git a/src/backend/PkgDiff.Tests/Outbound/RegistryUrlEscapingTests.cs b/src/backend/PkgDiff.Tests/Outbound/RegistryUrlEscapingTests.cs index 7ed4298..bca0a8c 100644 --- a/src/backend/PkgDiff.Tests/Outbound/RegistryUrlEscapingTests.cs +++ b/src/backend/PkgDiff.Tests/Outbound/RegistryUrlEscapingTests.cs @@ -113,8 +113,13 @@ public async Task PubDevService_Changelog_EscapesPackageId() await service.GetReleaseNotesAsync("../x", "1.0.0"); - var request = Assert.Single(handler.Requests); - Assert.Equal("https://pub.dev/packages/..%2Fx/changelog", request.AbsoluteUri); + Assert.Equal( + new[] + { + "https://pub.dev/packages/..%2Fx/versions/1.0.0/changelog", + "https://pub.dev/packages/..%2Fx/changelog" + }, + handler.Requests.Select(r => r.AbsoluteUri)); } [Fact] diff --git a/src/backend/PkgDiff.Tests/Smoke/RegistrySmokeTests.cs b/src/backend/PkgDiff.Tests/Smoke/RegistrySmokeTests.cs index 3ae2b26..63029df 100644 --- a/src/backend/PkgDiff.Tests/Smoke/RegistrySmokeTests.cs +++ b/src/backend/PkgDiff.Tests/Smoke/RegistrySmokeTests.cs @@ -197,7 +197,13 @@ public async Task PubDev_SmokeAnalysis() Assert.Equal(string.Empty, unknownNotes); Assert.Equal( - new[] { PubDevVersionsUrl, PubDevChangelogUrl, PubDevChangelogUrl, PubDevChangelogUrl }, + new[] + { + PubDevVersionsUrl, + PubDevVersionsUrl + "/1.6.0/changelog", PubDevChangelogUrl, + PubDevVersionsUrl + "/1.4.0/changelog", PubDevChangelogUrl, + PubDevVersionsUrl + "/0.0.1/changelog", PubDevChangelogUrl + }, handler.Requests); }