From c166526f2151b25ea2d79c38ba21216b20d9e1a8 Mon Sep 17 00:00:00 2001 From: Brian Hill Date: Wed, 19 Aug 2026 11:42:19 -0400 Subject: [PATCH 1/8] Scaffold DigitalOcean DNS-01 domain validator plugin Implements IDomainValidator against the DigitalOcean v2 API: Bearer token auth, zone discovery by longest domain-name suffix match, and TXT record create/delete keyed by DigitalOcean's numeric record id. Modeled on the LuaDNS reference plugin and the win-acme DigitalOcean validator for the proven create/delete call shape. --- .../workflows/keyfactor-starter-workflow.yml | 27 ++ .gitignore | 4 + CHANGELOG.md | 2 + .../DigitalOceanDomainValidatorTests.cs | 45 +++ .../DigitalOceanProviderTests.cs | 259 +++++++++++++++++ .../FakeHttpMessageHandler.cs | 35 +++ ...ctor.DnsProvider.DigitalOcean.Tests.csproj | 19 ++ Keyfactor.DnsProvider.DigitalOcean.slnx | 4 + .../AssemblyInfo.cs | 3 + .../DigitalOceanDomainValidator.cs | 121 ++++++++ .../DigitalOceanProvider.cs | 274 ++++++++++++++++++ .../Keyfactor.DnsProvider.DigitalOcean.csproj | 19 ++ .../manifest.json | 10 + docsource/configuration.md | 48 +++ docsource/content.md | 13 + integration-manifest.json | 36 +++ 16 files changed, 919 insertions(+) create mode 100644 .github/workflows/keyfactor-starter-workflow.yml create mode 100644 CHANGELOG.md create mode 100644 Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs create mode 100644 Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs create mode 100644 Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs create mode 100644 Keyfactor.DnsProvider.DigitalOcean.Tests/Keyfactor.DnsProvider.DigitalOcean.Tests.csproj create mode 100644 Keyfactor.DnsProvider.DigitalOcean.slnx create mode 100644 Keyfactor.DnsProvider.DigitalOcean/AssemblyInfo.cs create mode 100644 Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs create mode 100644 Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs create mode 100644 Keyfactor.DnsProvider.DigitalOcean/Keyfactor.DnsProvider.DigitalOcean.csproj create mode 100644 Keyfactor.DnsProvider.DigitalOcean/manifest.json create mode 100644 docsource/configuration.md create mode 100644 docsource/content.md create mode 100644 integration-manifest.json diff --git a/.github/workflows/keyfactor-starter-workflow.yml b/.github/workflows/keyfactor-starter-workflow.yml new file mode 100644 index 0000000..0f3d3ae --- /dev/null +++ b/.github/workflows/keyfactor-starter-workflow.yml @@ -0,0 +1,27 @@ +name: Keyfactor Bootstrap Workflow + +on: + workflow_dispatch: + pull_request: + types: [opened, closed, synchronize, edited, reopened] + push: + create: + branches: + - 'release-*.*' + +jobs: + call-starter-workflow: + uses: keyfactor/actions/.github/workflows/starter.yml@v5 + with: + command_token_url: ${{ vars.COMMAND_TOKEN_URL }} + command_hostname: ${{ vars.COMMAND_HOSTNAME }} + command_base_api_path: ${{ vars.COMMAND_API_PATH }} + secrets: + token: ${{ secrets.V2BUILDTOKEN}} + gpg_key: ${{ secrets.KF_GPG_PRIVATE_KEY }} + gpg_pass: ${{ secrets.KF_GPG_PASSPHRASE }} + scan_token: ${{ secrets.SAST_TOKEN }} + entra_username: ${{ secrets.DOCTOOL_ENTRA_USERNAME }} + entra_password: ${{ secrets.DOCTOOL_ENTRA_PASSWD }} + command_client_id: ${{ secrets.COMMAND_CLIENT_ID }} + command_client_secret: ${{ secrets.COMMAND_CLIENT_SECRET }} diff --git a/.gitignore b/.gitignore index d5a18de..3a9bb62 100644 --- a/.gitignore +++ b/.gitignore @@ -427,3 +427,7 @@ FodyWeavers.xsd *.msix *.msm *.msp + +# Claude Code / agent state, and local vendor secrets +.claude/ +.secrets/ diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..78d2335 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,2 @@ +v1.0.0 +- Inital Version diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs new file mode 100644 index 0000000..e7ef93a --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs @@ -0,0 +1,45 @@ +using Xunit; + +namespace Keyfactor.Extensions.DomainValidator.DigitalOcean.Tests +{ + public class DigitalOceanDomainValidatorTests + { + [Fact] + public void GetValidationType_ReturnsDns01() + { + var validator = new DigitalOceanDomainValidator(); + + Assert.Equal("dns-01", validator.GetValidationType()); + } + + [Fact] + public void GetDomainValidatorAnnotations_DeclaresApiToken() + { + var validator = new DigitalOceanDomainValidator(); + + var annotations = validator.GetDomainValidatorAnnotations(); + + Assert.True(annotations.ContainsKey("DigitalOcean_ApiToken")); + Assert.Equal("Secret", annotations["DigitalOcean_ApiToken"].Type); + Assert.True(annotations["DigitalOcean_ApiToken"].Hidden); + } + + [Fact] + public async Task ValidateConfiguration_ThrowsWhenApiTokenMissing() + { + var validator = new DigitalOceanDomainValidator(); + var config = new Dictionary(); + + await Assert.ThrowsAsync(() => validator.ValidateConfiguration(config)); + } + + [Fact] + public async Task ValidateConfiguration_SucceedsWhenApiTokenPresent() + { + var validator = new DigitalOceanDomainValidator(); + var config = new Dictionary { ["DigitalOcean_ApiToken"] = "token" }; + + await validator.ValidateConfiguration(config); + } + } +} diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs new file mode 100644 index 0000000..480e6ba --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs @@ -0,0 +1,259 @@ +using System.Net; +using Xunit; + +namespace Keyfactor.Extensions.DomainValidator.DigitalOcean.Tests +{ + public class DigitalOceanProviderTests + { + [Fact] + public void FindBestMatch_PicksLongestMatchingSuffix() + { + var zones = new[] { "com", "example.com", "other.com" }; + + var match = DigitalOceanProvider.FindBestMatch(zones, "_acme-challenge.www.example.com"); + + Assert.Equal("example.com", match); + } + + [Fact] + public void FindBestMatch_MatchesExactZoneName() + { + var zones = new[] { "example.com" }; + + var match = DigitalOceanProvider.FindBestMatch(zones, "example.com"); + + Assert.Equal("example.com", match); + } + + [Fact] + public void FindBestMatch_ReturnsNullWhenNoZoneMatches() + { + var zones = new[] { "example.com" }; + + var match = DigitalOceanProvider.FindBestMatch(zones, "unrelated-domain.net"); + + Assert.Null(match); + } + + [Fact] + public void FindBestMatch_DoesNotMatchUnrelatedSuffixSubstring() + { + // "notexample.com" must not match zone "example.com" just because it ends with the same characters. + var zones = new[] { "example.com" }; + + var match = DigitalOceanProvider.FindBestMatch(zones, "notexample.com"); + + Assert.Null(match); + } + + [Fact] + public void RelativeRecordName_StripsZoneSuffix() + { + var relative = DigitalOceanProvider.RelativeRecordName("example.com", "_acme-challenge.example.com"); + + Assert.Equal("_acme-challenge", relative); + } + + [Fact] + public void RelativeRecordName_ReturnsApexMarkerForZoneRoot() + { + var relative = DigitalOceanProvider.RelativeRecordName("example.com", "example.com"); + + Assert.Equal("@", relative); + } + + [Fact] + public async Task CreateRecordAsync_PostsTxtRecordToResolvedZone() + { + HttpRequestMessage postRequest = null; + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Post) + { + postRequest = req; + return FakeHttpMessageHandler.Json(HttpStatusCode.Created, + "{\"domain_record\":{\"id\":123,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"abc123\",\"ttl\":300}}"); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var result = await provider.CreateRecordAsync("_acme-challenge.example.com", "abc123", "TXT"); + + Assert.True(result); + Assert.NotNull(postRequest); + Assert.EndsWith("domains/example.com/records", postRequest.RequestUri.PathAndQuery); + + var body = await postRequest.Content.ReadAsStringAsync(); + Assert.Contains("\"name\":\"_acme-challenge\"", body); + Assert.Contains("\"type\":\"TXT\"", body); + Assert.Contains("\"data\":\"abc123\"", body); + } + + [Fact] + public async Task CreateRecordAsync_ThrowsWithApiDetailsWhenApiRejects() + { + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + return FakeHttpMessageHandler.Json(HttpStatusCode.BadRequest, "{\"id\":\"invalid_request\",\"message\":\"invalid data\"}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var ex = await Assert.ThrowsAsync( + () => provider.CreateRecordAsync("_acme-challenge.example.com", "abc123", "TXT")); + + Assert.Contains("400", ex.Message); + Assert.Contains("example.com", ex.Message); + } + + [Fact] + public async Task CreateRecordAsync_ThrowsWhenNoZoneMatches() + { + var handler = new FakeHttpMessageHandler(req => + FakeHttpMessageHandler.Json(HttpStatusCode.OK, "{\"domains\":[{\"name\":\"other.com\"}],\"links\":{}}")); + + var provider = new DigitalOceanProvider("token", handler); + + var ex = await Assert.ThrowsAsync( + () => provider.CreateRecordAsync("_acme-challenge.example.com", "abc123", "TXT")); + + Assert.Contains("No DigitalOcean domain found", ex.Message); + } + + [Fact] + public async Task CreateRecordAsync_ThrowsAuthErrorNamingLikelyCauseOn401() + { + var handler = new FakeHttpMessageHandler(req => + FakeHttpMessageHandler.Json(HttpStatusCode.Unauthorized, "{\"id\":\"unauthorized\",\"message\":\"Unable to authenticate you.\"}")); + + var provider = new DigitalOceanProvider("bad-token", handler); + + var ex = await Assert.ThrowsAsync( + () => provider.CreateRecordAsync("_acme-challenge.example.com", "abc123", "TXT")); + + Assert.Contains("401", ex.Message); + Assert.Contains("API token", ex.Message); + } + + [Fact] + public async Task CreateRecordAsync_FollowsPaginationToFindZone() + { + var handler = new FakeHttpMessageHandler(req => + { + if (req.RequestUri.PathAndQuery.Contains("page=2")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"other.com\"}],\"links\":{\"pages\":{\"next\":\"https://api.digitalocean.com/v2/domains?page=2&per_page=200\"}}}"); + } + + if (req.Method == HttpMethod.Post) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.Created, + "{\"domain_record\":{\"id\":123,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"abc123\",\"ttl\":300}}"); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var result = await provider.CreateRecordAsync("_acme-challenge.example.com", "abc123", "TXT"); + + Assert.True(result); + } + + [Fact] + public async Task DeleteRecordAsync_DeletesMatchingRecord() + { + HttpRequestMessage deleteRequest = null; + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[{\"id\":7,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"abc123\",\"ttl\":300}]}"); + } + + if (req.Method == HttpMethod.Delete) + { + deleteRequest = req; + return new HttpResponseMessage(HttpStatusCode.NoContent); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var result = await provider.DeleteRecordAsync("_acme-challenge.example.com", "TXT"); + + Assert.True(result); + Assert.NotNull(deleteRequest); + Assert.EndsWith("domains/example.com/records/7", deleteRequest.RequestUri.PathAndQuery); + } + + [Fact] + public async Task DeleteRecordAsync_IsIdempotentWhenRecordAlreadyGone() + { + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, "{\"domain_records\":[]}"); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var result = await provider.DeleteRecordAsync("_acme-challenge.example.com", "TXT"); + + Assert.True(result); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public void Constructor_ThrowsOnMissingApiToken(string apiToken) + { + Assert.Throws(() => new DigitalOceanProvider(apiToken, new FakeHttpMessageHandler(_ => + throw new InvalidOperationException("Should not make HTTP calls")))); + } + } +} diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs new file mode 100644 index 0000000..8d0c8e3 --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs @@ -0,0 +1,35 @@ +using System.Net; +using System.Net.Http; + +namespace Keyfactor.Extensions.DomainValidator.DigitalOcean.Tests +{ + /// + /// Routes requests to a caller-supplied responder so DigitalOceanProvider can be + /// exercised end-to-end without touching the real DigitalOcean API. + /// + internal class FakeHttpMessageHandler : HttpMessageHandler + { + public List Requests { get; } = new(); + + private readonly Func _responder; + + public FakeHttpMessageHandler(Func responder) + { + _responder = responder; + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + Requests.Add(request); + return Task.FromResult(_responder(request)); + } + + public static HttpResponseMessage Json(HttpStatusCode status, string body) + { + return new HttpResponseMessage(status) + { + Content = new StringContent(body, System.Text.Encoding.UTF8, "application/json") + }; + } + } +} diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/Keyfactor.DnsProvider.DigitalOcean.Tests.csproj b/Keyfactor.DnsProvider.DigitalOcean.Tests/Keyfactor.DnsProvider.DigitalOcean.Tests.csproj new file mode 100644 index 0000000..bf0f88c --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/Keyfactor.DnsProvider.DigitalOcean.Tests.csproj @@ -0,0 +1,19 @@ + + + net10.0 + enable + disable + false + Keyfactor.Extensions.DomainValidator.DigitalOcean.Tests + + + + + + + + + + + + diff --git a/Keyfactor.DnsProvider.DigitalOcean.slnx b/Keyfactor.DnsProvider.DigitalOcean.slnx new file mode 100644 index 0000000..2bcb933 --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean.slnx @@ -0,0 +1,4 @@ + + + + diff --git a/Keyfactor.DnsProvider.DigitalOcean/AssemblyInfo.cs b/Keyfactor.DnsProvider.DigitalOcean/AssemblyInfo.cs new file mode 100644 index 0000000..aca4153 --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Keyfactor.DnsProvider.DigitalOcean.Tests")] diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs new file mode 100644 index 0000000..fdb6a3f --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs @@ -0,0 +1,121 @@ +// Copyright 2026 Keyfactor +// Licensed under the Apache License, Version 2.0 +using Keyfactor.AnyGateway.Extensions; +using Keyfactor.Logging; +using Microsoft.Extensions.Logging; + +namespace Keyfactor.Extensions.DomainValidator.DigitalOcean +{ + /// + /// DigitalOcean domain validator for ACME DNS-01 challenges. Publishes TXT records + /// in DigitalOcean-hosted domains. Authenticates via a Bearer Personal Access Token. + /// + public class DigitalOceanDomainValidator : IDomainValidator + { + private static readonly ILogger _logger = LogHandler.GetClassLogger(); + + private const string ValidationTypeName = "dns-01"; + private const string RecordTypeName = "TXT"; + + private DigitalOceanProvider _provider; + private Dictionary _configuration; + + public Dictionary GetDomainValidatorAnnotations() + { + return new Dictionary() + { + ["DigitalOcean_ApiToken"] = new PropertyConfigInfo() + { + Comments = "DigitalOcean Personal Access Token with domain read/create/delete scopes (Required)", + Hidden = true, + DefaultValue = "", + Type = "Secret" + } + }; + } + + public string GetValidationType() => ValidationTypeName; + + public void Initialize(IDomainValidatorConfigProvider configProvider) + { + _configuration = configProvider.DomainValidationConfiguration; + + var apiToken = GetConfigValue("DigitalOcean_ApiToken"); + + if (string.IsNullOrWhiteSpace(apiToken)) + { + throw new ArgumentException("DigitalOcean_ApiToken is required"); + } + + _provider = new DigitalOceanProvider(apiToken); + } + + public async Task StageValidation(string key, string value, CancellationToken cancellationToken) + { + try + { + var success = await _provider.CreateRecordAsync(key, value, RecordTypeName); + + return new DomainValidationResult + { + Success = success, + ErrorMessage = success ? null : $"Failed to create DNS {RecordTypeName} record for {key}" + }; + } + catch (Exception ex) + { + _logger.LogError(ex, "DigitalOcean StageValidation failed for {RecordType} record '{Key}'", RecordTypeName, key); + return new DomainValidationResult + { + Success = false, + ErrorMessage = $"Failed to create {RecordTypeName} record for {key}: {ex.Message}" + }; + } + } + + public async Task CleanupValidation(string key, CancellationToken cancellationToken) + { + try + { + var success = await _provider.DeleteRecordAsync(key, RecordTypeName); + + return new DomainValidationResult + { + Success = success, + ErrorMessage = success ? null : $"Failed to delete DNS {RecordTypeName} record for {key}" + }; + } + catch (Exception ex) + { + _logger.LogError(ex, "DigitalOcean CleanupValidation failed for {RecordType} record '{Key}'", RecordTypeName, key); + return new DomainValidationResult + { + Success = false, + ErrorMessage = $"Failed to delete {RecordTypeName} record for {key}: {ex.Message}" + }; + } + } + + public async Task ValidateConfiguration(Dictionary configuration) + { + _configuration = configuration; + + var apiToken = GetConfigValue("DigitalOcean_ApiToken"); + if (string.IsNullOrWhiteSpace(apiToken)) + { + throw new ArgumentException("DigitalOcean_ApiToken is required"); + } + + await Task.CompletedTask; + } + + private string GetConfigValue(string key) + { + if (_configuration != null && _configuration.TryGetValue(key, out var value)) + { + return value?.ToString() ?? string.Empty; + } + return string.Empty; + } + } +} diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs new file mode 100644 index 0000000..a0d5d06 --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs @@ -0,0 +1,274 @@ +// Copyright 2026 Keyfactor +// Licensed under the Apache License, Version 2.0 +using System.Net.Http.Headers; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using Keyfactor.Logging; +using Microsoft.Extensions.Logging; + +namespace Keyfactor.Extensions.DomainValidator.DigitalOcean +{ + internal class DigitalOceanProvider + { + private static readonly ILogger _logger = LogHandler.GetClassLogger(); + + private readonly HttpClient _httpClient; + + private class DomainData + { + [JsonPropertyName("name")] + public string Name { get; set; } + } + + private class DomainsResponse + { + [JsonPropertyName("domains")] + public DomainData[] Domains { get; set; } + + [JsonPropertyName("links")] + public LinksData Links { get; set; } + } + + private class LinksData + { + [JsonPropertyName("pages")] + public PagesData Pages { get; set; } + } + + private class PagesData + { + [JsonPropertyName("next")] + public string Next { get; set; } + } + + private class RecordData + { + [JsonPropertyName("id")] + public long Id { get; set; } + + [JsonPropertyName("type")] + public string Type { get; set; } + + [JsonPropertyName("name")] + public string Name { get; set; } + + [JsonPropertyName("data")] + public string Data { get; set; } + + [JsonPropertyName("ttl")] + public int Ttl { get; set; } + } + + private class RecordsResponse + { + [JsonPropertyName("domain_records")] + public RecordData[] DomainRecords { get; set; } + } + + private class CreateRecordResponse + { + [JsonPropertyName("domain_record")] + public RecordData DomainRecord { get; set; } + } + + public DigitalOceanProvider(string apiToken) + : this(apiToken, new HttpClientHandler()) + { + } + + // Internal constructor to allow unit tests to inject a fake HttpMessageHandler. + internal DigitalOceanProvider(string apiToken, HttpMessageHandler handler) + { + if (string.IsNullOrWhiteSpace(apiToken)) + { + throw new ArgumentException("apiToken must not be empty", nameof(apiToken)); + } + + _httpClient = new HttpClient(handler) + { + BaseAddress = new Uri("https://api.digitalocean.com/v2/") + }; + + _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", apiToken); + _httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + } + + public async Task CreateRecordAsync(string recordName, string value, string recordType) + { + _logger.LogDebug("Creating {RecordType} record for {RecordName}", recordType, recordName); + + var zone = await FindZoneForRecordAsync(recordName); + var relativeName = RelativeRecordName(zone, recordName); + + var payload = new { type = recordType, name = relativeName, data = value, ttl = 300 }; + var json = JsonSerializer.Serialize(payload); + var content = new StringContent(json, Encoding.UTF8, "application/json"); + + var response = await _httpClient.PostAsync($"domains/{zone}/records", content); + var result = await response.Content.ReadAsStringAsync(); + + if (!response.IsSuccessStatusCode) + { + _logger.LogError( + "DigitalOcean API rejected creation of {RecordType} record '{RelativeName}' in zone '{Zone}'. Status: {StatusCode}. Response: {Response}", + recordType, relativeName, zone, (int)response.StatusCode, result); + + throw new InvalidOperationException( + $"DigitalOcean API returned {(int)response.StatusCode} ({response.StatusCode}) creating {recordType} record '{relativeName}' in zone '{zone}': {result}"); + } + + _logger.LogInformation( + "Created {RecordType} record '{RelativeName}' in DigitalOcean zone '{Zone}'", + recordType, relativeName, zone); + return true; + } + + public async Task DeleteRecordAsync(string recordName, string recordType) + { + _logger.LogDebug("Deleting {RecordType} record for {RecordName}", recordType, recordName); + + var zone = await FindZoneForRecordAsync(recordName); + var relativeName = RelativeRecordName(zone, recordName); + + var recordsResp = await _httpClient.GetAsync($"domains/{zone}/records?type={recordType}"); + var recordsBody = await recordsResp.Content.ReadAsStringAsync(); + if (!recordsResp.IsSuccessStatusCode) + { + _logger.LogError( + "DigitalOcean API failed to list records for zone '{Zone}'. Status: {StatusCode}. Response: {Response}", + zone, (int)recordsResp.StatusCode, recordsBody); + + throw new InvalidOperationException( + $"DigitalOcean API returned {(int)recordsResp.StatusCode} ({recordsResp.StatusCode}) listing records in zone '{zone}': {recordsBody}"); + } + + var records = JsonSerializer.Deserialize(recordsBody)?.DomainRecords ?? Array.Empty(); + var match = records.FirstOrDefault(r => + string.Equals(r.Type, recordType, StringComparison.OrdinalIgnoreCase) && + string.Equals(r.Name, relativeName, StringComparison.OrdinalIgnoreCase)); + + if (match == null) + { + // Nothing to clean up — treat as success so cleanup is idempotent. + _logger.LogInformation( + "No {RecordType} record '{RelativeName}' found in zone '{Zone}' to delete; treating cleanup as complete", + recordType, relativeName, zone); + return true; + } + + var deleteResp = await _httpClient.DeleteAsync($"domains/{zone}/records/{match.Id}"); + + if (!deleteResp.IsSuccessStatusCode) + { + var deleteBody = await deleteResp.Content.ReadAsStringAsync(); + _logger.LogError( + "DigitalOcean API rejected deletion of {RecordType} record '{RelativeName}' ({RecordId}) in zone '{Zone}'. Status: {StatusCode}. Response: {Response}", + recordType, relativeName, match.Id, zone, (int)deleteResp.StatusCode, deleteBody); + + throw new InvalidOperationException( + $"DigitalOcean API returned {(int)deleteResp.StatusCode} ({deleteResp.StatusCode}) deleting {recordType} record '{relativeName}' in zone '{zone}': {deleteBody}"); + } + + _logger.LogInformation( + "Deleted {RecordType} record '{RelativeName}' in DigitalOcean zone '{Zone}'", + recordType, relativeName, zone); + return true; + } + + /// + /// Fetches all domains (zones) on the account, paging through `links.pages.next`, and + /// resolves the zone that owns the given record by longest matching name suffix, e.g. + /// for "_acme-challenge.www.example.com" it tries "www.example.com", then "example.com". + /// + private async Task FindZoneForRecordAsync(string recordName) + { + if (string.IsNullOrWhiteSpace(recordName)) + { + throw new ArgumentException("Record name must not be empty", nameof(recordName)); + } + + var zoneNames = new List(); + var nextUri = "domains?per_page=200"; + + while (!string.IsNullOrEmpty(nextUri)) + { + var response = await _httpClient.GetAsync(nextUri); + var body = await response.Content.ReadAsStringAsync(); + + if (!response.IsSuccessStatusCode) + { + _logger.LogError( + "DigitalOcean domain list request failed. Status: {StatusCode}. Response: {Response}. " + + "This usually means the API token is invalid, expired, or missing the required scopes.", + (int)response.StatusCode, body); + + throw new InvalidOperationException( + $"DigitalOcean API returned {(int)response.StatusCode} ({response.StatusCode}) while listing domains: {body}. " + + "Verify the configured API token and its scopes."); + } + + var page = JsonSerializer.Deserialize(body); + var domains = page?.Domains ?? Array.Empty(); + zoneNames.AddRange(domains.Where(d => d.Name != null).Select(d => d.Name)); + + var next = page?.Links?.Pages?.Next; + nextUri = string.IsNullOrEmpty(next) ? null : new Uri(next).PathAndQuery.TrimStart('/'); + } + + if (zoneNames.Count == 0) + { + throw new InvalidOperationException("DigitalOcean returned an empty or invalid domains list. Aborting."); + } + + var match = FindBestMatch(zoneNames, recordName.TrimEnd('.')); + + if (match == null) + { + throw new InvalidOperationException( + $"No DigitalOcean domain found for record '{recordName}'. Ensure the domain exists in this DigitalOcean account."); + } + + return match; + } + + /// + /// Computes the record name relative to its owning zone, e.g. for zone "example.com" and + /// record "_acme-challenge.example.com" returns "_acme-challenge"; returns "@" when the + /// record name is the zone apex itself. + /// + internal static string RelativeRecordName(string zone, string recordName) + { + var trimmedRecord = recordName.TrimEnd('.'); + var trimmedZone = zone.TrimEnd('.'); + + if (trimmedRecord.Equals(trimmedZone, StringComparison.OrdinalIgnoreCase)) + { + return "@"; + } + + return trimmedRecord.Substring(0, trimmedRecord.Length - trimmedZone.Length - 1); + } + + /// + /// Finds the zone whose name is the longest suffix match of the target domain, + /// e.g. for domain "_acme-challenge.www.example.com" and zones {"example.com", "com"}, + /// "example.com" wins because it's the more specific (longer) match. + /// + internal static string FindBestMatch(IEnumerable zones, string domain) + { + string best = null; + foreach (var zone in zones) + { + var isMatch = domain.Equals(zone, StringComparison.OrdinalIgnoreCase) || + domain.EndsWith("." + zone, StringComparison.OrdinalIgnoreCase); + + if (isMatch && (best == null || zone.Length > best.Length)) + { + best = zone; + } + } + return best; + } + } +} diff --git a/Keyfactor.DnsProvider.DigitalOcean/Keyfactor.DnsProvider.DigitalOcean.csproj b/Keyfactor.DnsProvider.DigitalOcean/Keyfactor.DnsProvider.DigitalOcean.csproj new file mode 100644 index 0000000..e83722f --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean/Keyfactor.DnsProvider.DigitalOcean.csproj @@ -0,0 +1,19 @@ + + + net10.0 + enable + disable + true + Keyfactor.Extensions.DomainValidator.DigitalOcean + DigitalOceanDomainValidator + + + + + + + + Always + + + diff --git a/Keyfactor.DnsProvider.DigitalOcean/manifest.json b/Keyfactor.DnsProvider.DigitalOcean/manifest.json new file mode 100644 index 0000000..0dbbaac --- /dev/null +++ b/Keyfactor.DnsProvider.DigitalOcean/manifest.json @@ -0,0 +1,10 @@ +{ + "extensions": { + "Keyfactor.AnyGateway.Extensions.IDomainValidator": { + "DigitalOceanDomainValidator": { + "assemblypath": "DigitalOceanDomainValidator.dll", + "TypeFullName": "Keyfactor.Extensions.DomainValidator.DigitalOcean.DigitalOceanDomainValidator" + } + } + } +} diff --git a/docsource/configuration.md b/docsource/configuration.md new file mode 100644 index 0000000..68300d5 --- /dev/null +++ b/docsource/configuration.md @@ -0,0 +1,48 @@ +### Provider Setup + +Create a DigitalOcean Personal Access Token for the account that owns the domains you want the plugin to manage: + +1. Log in to the DigitalOcean control panel +2. Navigate to **API > Tokens** +3. Generate a new token with `domain:read`, `domain:create`, and `domain:delete` scopes and copy the value + +Provide the token as `DigitalOcean_ApiToken` in the plugin configuration below. + +### Example Configurations + +**Standard configuration:** + +```json +{ + "DigitalOcean_ApiToken": "your-digitalocean-api-token" +} +``` + +### Zone Discovery + +The plugin discovers the appropriate DigitalOcean domain for a record by querying the DigitalOcean API for all domains on the account, then matching the record's domain against domain names from most specific (longest) to least specific. + +### Testing Connectivity + +Test DigitalOcean connectivity using `curl` against the API: + +```bash +# List domains accessible to the account (validates the API token) +curl -s -H "Authorization: Bearer $DIGITALOCEAN_API_TOKEN" https://api.digitalocean.com/v2/domains +``` + +### Troubleshooting + +**Authentication Failures** + +Symptom: `401 Unauthorized` listing domains + +- Verify the API token has not expired or been revoked in the DigitalOcean control panel +- Confirm the token has `domain:read`, `domain:create`, and `domain:delete` scopes (or is a full-access token) + +**Zone Not Found** + +Symptom: `No DigitalOcean domain found for example.com` + +- Verify the domain exists and is active in the DigitalOcean account +- Confirm the account associated with the API token owns that domain diff --git a/docsource/content.md b/docsource/content.md new file mode 100644 index 0000000..c9edc7b --- /dev/null +++ b/docsource/content.md @@ -0,0 +1,13 @@ +## Overview + +The DigitalOcean Provider plugin enables automated DNS-based domain validation for Keyfactor certificate lifecycle management through DigitalOcean. This plugin integrates with the DigitalOcean API to automatically create, verify, and delete DNS TXT records required for domain validation during certificate issuance and renewal. + +## Features + +- Bearer token authentication using a DigitalOcean Personal Access Token +- Automatic zone discovery across all domains on the account, matched by longest domain suffix + +## Requirements + +- A DigitalOcean account with one or more domains managed by DigitalOcean's DNS +- A DigitalOcean Personal Access Token with `domain:read`, `domain:create`, and `domain:delete` scopes (create under **API > Tokens** in the DigitalOcean control panel) diff --git a/integration-manifest.json b/integration-manifest.json new file mode 100644 index 0000000..e465d46 --- /dev/null +++ b/integration-manifest.json @@ -0,0 +1,36 @@ +{ + "$schema": "https://keyfactor.github.io/v2/integration-manifest-schema.json", + "integration_type": "dns-plugin", + "name": "DigitalOcean DNS Plugin", + "status": "production", + "support_level": "kf-supported", + "update_catalog": true, + "link_github": false, + "description": "DNS-01 challenge validation provider using DigitalOcean. Implements the IDomainValidator interface to create, manage, and clean up DNS TXT records in DigitalOcean-hosted domains for ACME domain validation. Authenticates via a Bearer Personal Access Token.", + "release_dir": "Keyfactor.DnsProvider.DigitalOcean/bin/Release", + "release_project": "Keyfactor.DnsProvider.DigitalOcean/Keyfactor.DnsProvider.DigitalOcean.csproj", + "about": { + "dns_provider": { + "providerName": "digitalocean", + "displayName": "DigitalOcean", + "assemblyName": "DigitalOceanDomainValidator", + "fullyQualifiedClassName": "Keyfactor.Extensions.DomainValidator.DigitalOcean.DigitalOceanDomainValidator", + "validationType": "dns-01", + "providerEndpoint": "api.digitalocean.com", + "providerDocsUrl": "https://docs.digitalocean.com/reference/api/reference/domains/", + "serviceStatusUrl": "https://status.digitalocean.com/", + "dns_provider_config": [ + { + "Name": "DigitalOcean_ApiToken", + "DisplayName": "DigitalOcean API Token", + "DataType": 2, + "InstanceLevel": false, + "Hidden": true, + "DefaultValue": "", + "Required": true, + "Description": "DigitalOcean Personal Access Token with domain read/create/delete scopes. Created under API > Tokens in the DigitalOcean control panel." + } + ] + } + } +} From f6b3582ac7207428c7bd90faddebbc11edf1dde9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 19 Aug 2026 15:46:07 +0000 Subject: [PATCH 2/8] docs: auto-generate README and documentation [skip ci] --- README.md | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 0000000..febc496 --- /dev/null +++ b/README.md @@ -0,0 +1,202 @@ +

+ DigitalOcean DNS Provider +

+ +

+ +Integration Status: production +Build +Release +Issues +GitHub Downloads (all assets, all releases) +

+ +

+ + + Support + + · + + Requirements + + · + + Installation + + · + + License + + · + + Related Integrations + +

+ +## Overview + +The DigitalOcean Provider plugin enables automated DNS-based domain validation for Keyfactor certificate lifecycle management through DigitalOcean. This plugin integrates with the DigitalOcean API to automatically create, verify, and delete DNS TXT records required for domain validation during certificate issuance and renewal. + +## Features + +- Automated DNS TXT record creation and deletion in DigitalOcean +- Bearer token authentication using a DigitalOcean Personal Access Token +- Automatic zone discovery across all domains on the account, matched by longest domain suffix + +## Requirements + +### Keyfactor Platform +- Keyfactor AnyCA Gateway REST **26.2 or later** (DNS validation support was added in AnyCA Gateway 26.2) +- A gateway product that supports DNS-01 domain validation (ACME REST Gateway, DigiCert, Sectigo, etc.) + +### DigitalOcean Requirements + +- A DigitalOcean account with one or more domains managed by DigitalOcean's DNS +- A DigitalOcean Personal Access Token with `domain:read`, `domain:create`, and `domain:delete` scopes (create under **API > Tokens** in the DigitalOcean control panel) + +### Runtime Requirements +- .NET 10.0 runtime (provided by the gateway server) +- Network connectivity to api.digitalocean.com (HTTPS/443) + +## Installation + +This plugin is installed alongside any Keyfactor gateway server that supports DNS-01 domain validation (ACME REST Gateway, DigiCert, Sectigo, etc.). The same DLL works with every supported gateway. + +> See the official Keyfactor AnyCA Gateway REST installation documentation for the authoritative install instructions: ****. The steps below are a general guide; defer to the official docs if they diverge. + +### 1. Download the Plugin + +Download the latest release from the [Releases](https://github.com/Keyfactor/digitalocean-dnsplugin/releases) page. + +### 2. Copy the plugin DLLs to the gateway's Extensions folder + +On the server hosting your gateway, unzip the release and copy the contents of the `net10.0` directory into the gateway's `Extensions` folder. + +**Windows** (example path — substitute the gateway product folder for your install): + +```text +C:\Program Files\Keyfactor\\AnyGatewayREST\net10.0\Extensions\ +``` + +**Linux**: + +```text +/opt/keyfactor//AnyGatewayREST/net10.0/Extensions/ +``` + +Replace `` (or `` on Linux) with the gateway you are installing into (e.g. `AcmeGwDns`, `DigiCert`, `Sectigo`). + +### 3. Restart the gateway service + +Restart the AnyGatewayREST Windows service for the gateway you installed the plugin into so the Extensions folder is rescanned. + +## Configuration + +After installing the plugin DLL into the gateway's Extensions folder, configure a new DNS Provider entry in the AnyCA Gateway REST UI and select **DigitalOcean** as the provider type. See the official Keyfactor AnyCA Gateway REST documentation for the canonical UI walkthrough: ****. + +### DigitalOcean Setup + +Create a DigitalOcean Personal Access Token for the account that owns the domains you want the plugin to manage: + +1. Log in to the DigitalOcean control panel +2. Navigate to **API > Tokens** +3. Generate a new token with `domain:read`, `domain:create`, and `domain:delete` scopes and copy the value + +Provide the token as `DigitalOcean_ApiToken` in the plugin configuration below. + +### Configuration Parameters + +| Parameter | Description | Required | Example | +|-----------|-------------|----------|---------| +| `DigitalOcean_ApiToken` | DigitalOcean Personal Access Token with domain read/create/delete scopes. Created under API > Tokens in the DigitalOcean control panel. | Yes | ` ` | + +### Example Configuration + +**Standard configuration:** + +```json +{ + "DigitalOcean_ApiToken": "your-digitalocean-api-token" +} +``` + +## Usage + +### Automatic Domain Validation + +Once configured, the plugin automatically handles DNS validation during certificate enrollment and renewal: + +1. **Record Creation**: Plugin creates a DNS TXT record with the validation challenge +2. **Propagation Wait**: Plugin waits for DNS propagation +3. **Verification**: Plugin verifies the record exists on DigitalOcean nameservers +4. **Cleanup**: Plugin deletes the validation record after successful validation + +### Zone Discovery + +The plugin discovers the appropriate DigitalOcean domain for a record by querying the DigitalOcean API for all domains on the account, then matching the record's domain against domain names from most specific (longest) to least specific. + +### Testing Connectivity + +Test DigitalOcean connectivity using `curl` against the API: + +```bash +# List domains accessible to the account (validates the API token) +curl -s -H "Authorization: Bearer $DIGITALOCEAN_API_TOKEN" https://api.digitalocean.com/v2/domains +``` + +## Troubleshooting + +### Common Issues + +**Authentication Failures** + +Symptom: `401 Unauthorized` listing domains + +- Verify the API token has not expired or been revoked in the DigitalOcean control panel +- Confirm the token has `domain:read`, `domain:create`, and `domain:delete` scopes (or is a full-access token) + +**Zone Not Found** + +Symptom: `No DigitalOcean domain found for example.com` + +- Verify the domain exists and is active in the DigitalOcean account +- Confirm the account associated with the API token owns that domain + +### Logging + +Enable debug logging in the gateway's logging configuration: + +```json +{ + "Logging": { + "LogLevel": { + "Keyfactor.Extensions.DomainValidator.DigitalOcean": "Debug" + } + } +} +``` + +### Service Status + +Check DigitalOcean service status: https://status.digitalocean.com/ + +## Support + +The DigitalOcean DNS Provider plugin is supported by Keyfactor for Keyfactor customers. If you have a support issue, please open a support ticket via the Keyfactor Support Portal at https://support.keyfactor.com. + +### Resources + +- [DigitalOcean Documentation](https://docs.digitalocean.com/reference/api/reference/domains/) +- [Report Issues](https://github.com/Keyfactor/digitalocean-dnsplugin/issues) +- [Discussions](https://github.com/Keyfactor/digitalocean-dnsplugin/discussions) + +> To report a problem or suggest a new feature, use the **[Issues](../../issues)** tab. If you want to contribute actual bug fixes or proposed enhancements, use the **[Pull requests](../../pulls)** tab. + +## License + +Apache License 2.0, see [LICENSE](LICENSE). + +## Related Integrations + +See all [Keyfactor DNS Provider plugins](https://github.com/orgs/Keyfactor/repositories?q=dnsplugin). From 9d11bafd05fe28313679aec0c4afd701af174b20 Mon Sep 17 00:00:00 2001 From: Sean <1661003+spbsoluble@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:50:33 -0700 Subject: [PATCH 3/8] fix: pagination double-v2 path, cleanup value collision, cancellation, log-forging Full-review triage on PR #1: - FindZoneForRecordAsync used the pagination link's PathAndQuery instead of the absolute URL, doubling the /v2 segment on page 2+ and breaking zone resolution for any account with more than 200 domains. - DeleteRecordAsync matched by name+type only, so cleanup for one authorization could delete a different pending authorization's TXT record when two SANs (e.g. apex + wildcard) share the same _acme-challenge name. StageValidation now tracks staged values per key so CleanupValidation can match by value too. - CancellationToken accepted by StageValidation/CleanupValidation was never forwarded to the provider or its HTTP calls. - Record names reached log/exception messages with no control-character stripping, allowing embedded CRLF to forge log lines (CWE-117). --- .../DigitalOceanDomainValidatorTests.cs | 59 ++++++++ .../DigitalOceanProviderTests.cs | 130 +++++++++++++++++- .../FakeHttpMessageHandler.cs | 3 + .../DigitalOceanDomainValidator.cs | 49 ++++++- .../DigitalOceanProvider.cs | 71 +++++++--- 5 files changed, 285 insertions(+), 27 deletions(-) diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs index e7ef93a..ca04223 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs @@ -1,3 +1,4 @@ +using System.Net; using Xunit; namespace Keyfactor.Extensions.DomainValidator.DigitalOcean.Tests @@ -41,5 +42,63 @@ public async Task ValidateConfiguration_SucceedsWhenApiTokenPresent() await validator.ValidateConfiguration(config); } + + [Fact] + public async Task CleanupValidation_DeletesTheStagedValue_NotJustTheFirstSameNameRecord() + { + // Simulates an apex + wildcard SAN pair: both authorizations challenge at the identical + // _acme-challenge FQDN with different values, staged in order [value-A, value-B]. The + // DigitalOcean records list is returned in the OPPOSITE order on purpose — a name-only + // match (the pre-fix behavior) would delete value-B's record on the first cleanup call + // even though value-A's authorization is the one being cleaned up. + var deletedIds = new List(); + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Post) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.Created, + "{\"domain_record\":{\"id\":1,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"ignored\",\"ttl\":300}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[" + + "{\"id\":11,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"value-B\",\"ttl\":300}," + + "{\"id\":10,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"value-A\",\"ttl\":300}]}"); + } + + if (req.Method == HttpMethod.Delete) + { + deletedIds.Add(req.RequestUri.PathAndQuery.Split('/').Last()); + return new HttpResponseMessage(HttpStatusCode.NoContent); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + const string key = "_acme-challenge.example.com"; + var stageA = await validator.StageValidation(key, "value-A", CancellationToken.None); + var stageB = await validator.StageValidation(key, "value-B", CancellationToken.None); + Assert.True(stageA.Success); + Assert.True(stageB.Success); + + var cleanupA = await validator.CleanupValidation(key, CancellationToken.None); + var cleanupB = await validator.CleanupValidation(key, CancellationToken.None); + + Assert.True(cleanupA.Success); + Assert.True(cleanupB.Success); + Assert.Equal(new[] { "10", "11" }, deletedIds); + } } } diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs index 480e6ba..7c0e2f8 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs @@ -154,18 +154,24 @@ public async Task CreateRecordAsync_ThrowsAuthErrorNamingLikelyCauseOn401() [Fact] public async Task CreateRecordAsync_FollowsPaginationToFindZone() { + // Distinguishes page 1 vs page 2 by call order rather than a "page=2" substring match — + // "per_page=200" itself contains that substring, which previously made the first request + // match the "page 2" branch and let a doubled "/v2/v2/..." request URI go undetected. + var domainsRequests = new List(); + var handler = new FakeHttpMessageHandler(req => { - if (req.RequestUri.PathAndQuery.Contains("page=2")) + if (req.Method == HttpMethod.Get && req.RequestUri.AbsolutePath.Equals("/v2/domains", StringComparison.OrdinalIgnoreCase)) { - return FakeHttpMessageHandler.Json(HttpStatusCode.OK, - "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); - } + domainsRequests.Add(req); + if (domainsRequests.Count == 1) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"other.com\"}],\"links\":{\"pages\":{\"next\":\"https://api.digitalocean.com/v2/domains?page=2&per_page=200\"}}}"); + } - if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) - { return FakeHttpMessageHandler.Json(HttpStatusCode.OK, - "{\"domains\":[{\"name\":\"other.com\"}],\"links\":{\"pages\":{\"next\":\"https://api.digitalocean.com/v2/domains?page=2&per_page=200\"}}}"); + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); } if (req.Method == HttpMethod.Post) @@ -182,6 +188,8 @@ public async Task CreateRecordAsync_FollowsPaginationToFindZone() var result = await provider.CreateRecordAsync("_acme-challenge.example.com", "abc123", "TXT"); Assert.True(result); + Assert.Equal(2, domainsRequests.Count); + Assert.Equal("https://api.digitalocean.com/v2/domains?page=2&per_page=200", domainsRequests[1].RequestUri.ToString()); } [Fact] @@ -255,5 +263,113 @@ public void Constructor_ThrowsOnMissingApiToken(string apiToken) Assert.Throws(() => new DigitalOceanProvider(apiToken, new FakeHttpMessageHandler(_ => throw new InvalidOperationException("Should not make HTTP calls")))); } + + [Fact] + public async Task DeleteRecordAsync_WithExpectedValue_DeletesOnlyTheMatchingRecord() + { + // Simulates two TXT records sharing the same name (e.g. an apex + wildcard SAN both + // challenging at the same _acme-challenge FQDN with different values) — the delete must + // target the record whose value matches, not just the first record with that name. + HttpRequestMessage deleteRequest = null; + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[" + + "{\"id\":7,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"first-value\",\"ttl\":300}," + + "{\"id\":8,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"second-value\",\"ttl\":300}]}"); + } + + if (req.Method == HttpMethod.Delete) + { + deleteRequest = req; + return new HttpResponseMessage(HttpStatusCode.NoContent); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var result = await provider.DeleteRecordAsync("_acme-challenge.example.com", "TXT", "second-value"); + + Assert.True(result); + Assert.NotNull(deleteRequest); + Assert.EndsWith("domains/example.com/records/8", deleteRequest.RequestUri.PathAndQuery); + } + + [Fact] + public async Task DeleteRecordAsync_WithExpectedValue_IsIdempotentWhenNoRecordMatchesTheValue() + { + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[{\"id\":7,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"other-value\",\"ttl\":300}]}"); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var result = await provider.DeleteRecordAsync("_acme-challenge.example.com", "TXT", "expected-value"); + + Assert.True(result); + } + + [Fact] + public async Task CreateRecordAsync_ObservesCancellationToken() + { + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + var handler = new FakeHttpMessageHandler(_ => + throw new InvalidOperationException("HTTP call should not be made when the token is already canceled")); + + var provider = new DigitalOceanProvider("token", handler); + + await Assert.ThrowsAnyAsync( + () => provider.CreateRecordAsync("_acme-challenge.example.com", "abc123", "TXT", cts.Token)); + } + + [Fact] + public async Task DeleteRecordAsync_ObservesCancellationToken() + { + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + var handler = new FakeHttpMessageHandler(_ => + throw new InvalidOperationException("HTTP call should not be made when the token is already canceled")); + + var provider = new DigitalOceanProvider("token", handler); + + await Assert.ThrowsAnyAsync( + () => provider.DeleteRecordAsync("_acme-challenge.example.com", "TXT", cancellationToken: cts.Token)); + } + + [Theory] + [InlineData("_acme-challenge.example.com\r\nFORGED LOG LINE", "_acme-challenge.example.comFORGED LOG LINE")] + [InlineData("plain.example.com", "plain.example.com")] + [InlineData(null, null)] + public void StripControlCharacters_RemovesControlCharactersOnly(string input, string expected) + { + Assert.Equal(expected, DigitalOceanProvider.StripControlCharacters(input)); + } } } diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs index 8d0c8e3..64fff62 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs @@ -20,6 +20,9 @@ public FakeHttpMessageHandler(Func resp protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { + // Real HttpMessageHandlers honor the token before doing any work; matching that here + // lets tests verify a caller's CancellationToken actually reaches the HTTP layer. + cancellationToken.ThrowIfCancellationRequested(); Requests.Add(request); return Task.FromResult(_responder(request)); } diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs index fdb6a3f..7176466 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs @@ -20,6 +20,27 @@ public class DigitalOceanDomainValidator : IDomainValidator private DigitalOceanProvider _provider; private Dictionary _configuration; + // Tracks the value staged for each key so CleanupValidation can disambiguate between + // multiple TXT records sharing the same name (e.g. an apex + wildcard SAN both challenging + // at the same _acme-challenge FQDN). CleanupValidation's own signature (key only, no value) + // can't tell us which record to delete, so this queue records staging order per key on a + // best-effort FIFO basis: cleanup for a key removes the oldest still-pending value staged + // for it. A lock guards concurrent Stage/Cleanup calls across SANs on the same instance. + private readonly Dictionary> _stagedValues = new(); + private readonly object _stagedValuesLock = new(); + + public DigitalOceanDomainValidator() + { + } + + // Internal constructor to allow unit tests to inject a fake provider without going through + // Initialize (which requires a real IDomainValidatorConfigProvider and constructs its own + // DigitalOceanProvider from a config-supplied API token). + internal DigitalOceanDomainValidator(DigitalOceanProvider provider) + { + _provider = provider; + } + public Dictionary GetDomainValidatorAnnotations() { return new Dictionary() @@ -44,6 +65,7 @@ public void Initialize(IDomainValidatorConfigProvider configProvider) if (string.IsNullOrWhiteSpace(apiToken)) { + _logger.LogWarning("DigitalOcean_ApiToken is missing or empty; plugin initialization cannot proceed"); throw new ArgumentException("DigitalOcean_ApiToken is required"); } @@ -54,7 +76,20 @@ public async Task StageValidation(string key, string val { try { - var success = await _provider.CreateRecordAsync(key, value, RecordTypeName); + var success = await _provider.CreateRecordAsync(key, value, RecordTypeName, cancellationToken); + + if (success) + { + lock (_stagedValuesLock) + { + if (!_stagedValues.TryGetValue(key, out var queue)) + { + queue = new Queue(); + _stagedValues[key] = queue; + } + queue.Enqueue(value); + } + } return new DomainValidationResult { @@ -77,7 +112,16 @@ public async Task CleanupValidation(string key, Cancella { try { - var success = await _provider.DeleteRecordAsync(key, RecordTypeName); + string expectedValue = null; + lock (_stagedValuesLock) + { + if (_stagedValues.TryGetValue(key, out var queue) && queue.Count > 0) + { + expectedValue = queue.Dequeue(); + } + } + + var success = await _provider.DeleteRecordAsync(key, RecordTypeName, expectedValue, cancellationToken); return new DomainValidationResult { @@ -103,6 +147,7 @@ public async Task ValidateConfiguration(Dictionary configuration var apiToken = GetConfigValue("DigitalOcean_ApiToken"); if (string.IsNullOrWhiteSpace(apiToken)) { + _logger.LogWarning("DigitalOcean_ApiToken is missing or empty; configuration validation failed"); throw new ArgumentException("DigitalOcean_ApiToken is required"); } diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs index a0d5d06..fe028c8 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs @@ -94,19 +94,20 @@ internal DigitalOceanProvider(string apiToken, HttpMessageHandler handler) _httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); } - public async Task CreateRecordAsync(string recordName, string value, string recordType) + public async Task CreateRecordAsync(string recordName, string value, string recordType, CancellationToken cancellationToken = default) { + recordName = StripControlCharacters(recordName); _logger.LogDebug("Creating {RecordType} record for {RecordName}", recordType, recordName); - var zone = await FindZoneForRecordAsync(recordName); + var zone = await FindZoneForRecordAsync(recordName, cancellationToken); var relativeName = RelativeRecordName(zone, recordName); var payload = new { type = recordType, name = relativeName, data = value, ttl = 300 }; var json = JsonSerializer.Serialize(payload); var content = new StringContent(json, Encoding.UTF8, "application/json"); - var response = await _httpClient.PostAsync($"domains/{zone}/records", content); - var result = await response.Content.ReadAsStringAsync(); + var response = await _httpClient.PostAsync($"domains/{zone}/records", content, cancellationToken); + var result = await response.Content.ReadAsStringAsync(cancellationToken); if (!response.IsSuccessStatusCode) { @@ -124,15 +125,16 @@ public async Task CreateRecordAsync(string recordName, string value, strin return true; } - public async Task DeleteRecordAsync(string recordName, string recordType) + public async Task DeleteRecordAsync(string recordName, string recordType, string expectedValue = null, CancellationToken cancellationToken = default) { + recordName = StripControlCharacters(recordName); _logger.LogDebug("Deleting {RecordType} record for {RecordName}", recordType, recordName); - var zone = await FindZoneForRecordAsync(recordName); + var zone = await FindZoneForRecordAsync(recordName, cancellationToken); var relativeName = RelativeRecordName(zone, recordName); - var recordsResp = await _httpClient.GetAsync($"domains/{zone}/records?type={recordType}"); - var recordsBody = await recordsResp.Content.ReadAsStringAsync(); + var recordsResp = await _httpClient.GetAsync($"domains/{zone}/records?type={recordType}", cancellationToken); + var recordsBody = await recordsResp.Content.ReadAsStringAsync(cancellationToken); if (!recordsResp.IsSuccessStatusCode) { _logger.LogError( @@ -144,9 +146,20 @@ public async Task DeleteRecordAsync(string recordName, string recordType) } var records = JsonSerializer.Deserialize(recordsBody)?.DomainRecords ?? Array.Empty(); - var match = records.FirstOrDefault(r => - string.Equals(r.Type, recordType, StringComparison.OrdinalIgnoreCase) && - string.Equals(r.Name, relativeName, StringComparison.OrdinalIgnoreCase)); + + // When multiple records share the same name/type (e.g. an apex + wildcard SAN both + // challenging at the same _acme-challenge FQDN with different values), matching by + // value as well prevents deleting a sibling authorization's still-pending record. + // expectedValue is only known when the caller staged it itself; without it we fall + // back to the original name/type-only match to preserve existing single-record behavior. + var match = expectedValue != null + ? records.FirstOrDefault(r => + string.Equals(r.Type, recordType, StringComparison.OrdinalIgnoreCase) && + string.Equals(r.Name, relativeName, StringComparison.OrdinalIgnoreCase) && + string.Equals(r.Data, expectedValue, StringComparison.Ordinal)) + : records.FirstOrDefault(r => + string.Equals(r.Type, recordType, StringComparison.OrdinalIgnoreCase) && + string.Equals(r.Name, relativeName, StringComparison.OrdinalIgnoreCase)); if (match == null) { @@ -157,11 +170,11 @@ public async Task DeleteRecordAsync(string recordName, string recordType) return true; } - var deleteResp = await _httpClient.DeleteAsync($"domains/{zone}/records/{match.Id}"); + var deleteResp = await _httpClient.DeleteAsync($"domains/{zone}/records/{match.Id}", cancellationToken); if (!deleteResp.IsSuccessStatusCode) { - var deleteBody = await deleteResp.Content.ReadAsStringAsync(); + var deleteBody = await deleteResp.Content.ReadAsStringAsync(cancellationToken); _logger.LogError( "DigitalOcean API rejected deletion of {RecordType} record '{RelativeName}' ({RecordId}) in zone '{Zone}'. Status: {StatusCode}. Response: {Response}", recordType, relativeName, match.Id, zone, (int)deleteResp.StatusCode, deleteBody); @@ -181,7 +194,7 @@ public async Task DeleteRecordAsync(string recordName, string recordType) /// resolves the zone that owns the given record by longest matching name suffix, e.g. /// for "_acme-challenge.www.example.com" it tries "www.example.com", then "example.com". /// - private async Task FindZoneForRecordAsync(string recordName) + private async Task FindZoneForRecordAsync(string recordName, CancellationToken cancellationToken = default) { if (string.IsNullOrWhiteSpace(recordName)) { @@ -193,8 +206,15 @@ private async Task FindZoneForRecordAsync(string recordName) while (!string.IsNullOrEmpty(nextUri)) { - var response = await _httpClient.GetAsync(nextUri); - var body = await response.Content.ReadAsStringAsync(); + // `next` (when present) is an ABSOLUTE URL from DigitalOcean's HATEOAS-style + // pagination, e.g. "https://api.digitalocean.com/v2/domains?page=2&per_page=200". + // HttpClient.GetAsync uses an absolute URI as-is, ignoring BaseAddress, so passing + // it straight through resolves correctly; re-deriving a relative path from it + // (previously done via Uri.PathAndQuery.TrimStart('/')) reintroduces the "/v2" + // segment and, combined with BaseAddress already ending in "/v2/", doubles it into + // "/v2/v2/...", which the real API 404s. + var response = await _httpClient.GetAsync(nextUri, cancellationToken); + var body = await response.Content.ReadAsStringAsync(cancellationToken); if (!response.IsSuccessStatusCode) { @@ -212,8 +232,7 @@ private async Task FindZoneForRecordAsync(string recordName) var domains = page?.Domains ?? Array.Empty(); zoneNames.AddRange(domains.Where(d => d.Name != null).Select(d => d.Name)); - var next = page?.Links?.Pages?.Next; - nextUri = string.IsNullOrEmpty(next) ? null : new Uri(next).PathAndQuery.TrimStart('/'); + nextUri = page?.Links?.Pages?.Next; } if (zoneNames.Count == 0) @@ -250,6 +269,22 @@ internal static string RelativeRecordName(string zone, string recordName) return trimmedRecord.Substring(0, trimmedRecord.Length - trimmedZone.Length - 1); } + /// + /// Strips control characters (including CR/LF) from a gateway-supplied record name before + /// it can reach any log message or exception text. A legitimate hostname never contains + /// these characters, so this only affects malformed/malicious input — it prevents an + /// unvalidated record name from forging log lines (CWE-117) via embedded CRLF. + /// + internal static string StripControlCharacters(string value) + { + if (string.IsNullOrEmpty(value)) + { + return value; + } + + return new string(value.Where(c => !char.IsControl(c)).ToArray()); + } + /// /// Finds the zone whose name is the longest suffix match of the target domain, /// e.g. for domain "_acme-challenge.www.example.com" and zones {"example.com", "com"}, From a036ee90673589e3c6b2c6b8afd6f6c2974c1de9 Mon Sep 17 00:00:00 2001 From: Sean <1661003+spbsoluble@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:10:16 -0700 Subject: [PATCH 4/8] fix: transactional cleanup dequeue, validator-layer log sanitization, pagination cap Round 2 full-review findings on the round-1 fix: - CleanupValidation popped the staged value from the FIFO queue before confirming the delete succeeded, permanently losing it on a failed/retried cleanup and corrupting disambiguation for later calls on the same key. Now peeks the value and only dequeues it after DeleteRecordAsync confirms success. - StripControlCharacters was only applied inside DigitalOceanProvider on a local copy of the record name; DigitalOceanDomainValidator's own log calls and DomainValidationResult.ErrorMessage still embedded the raw, unsanitized key (same CWE-117 gap one layer up). Added a SafeForLog wrapper at every validator call site that logs or returns the key. - FindZoneForRecordAsync's pagination loop had no bound; added a page cap so a malformed/cyclic `next` link can't hang validation indefinitely. - DeleteRecordAsync's success log now includes the record ID and whether the match was by value or by name, so an audit trail can distinguish a precise disambiguated delete from a name-only fallback. Residual, accepted risk: cleanup's best-effort FIFO match (oldest staged value) can pick the wrong record if two SANs sharing a challenge name complete out of staging order, since CleanupValidation's interface never receives the challenge value. Kept as the least-bad option (discussed and confirmed with the requester) and now logs a warning whenever more than one value is outstanding for a key, so the residual ambiguity is operationally visible. --- .../DigitalOceanDomainValidatorTests.cs | 79 +++++++++++++++++++ .../DigitalOceanDomainValidator.cs | 57 +++++++++++-- .../DigitalOceanProvider.cs | 17 +++- 3 files changed, 144 insertions(+), 9 deletions(-) diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs index ca04223..5521a2d 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs @@ -100,5 +100,84 @@ public async Task CleanupValidation_DeletesTheStagedValue_NotJustTheFirstSameNam Assert.True(cleanupB.Success); Assert.Equal(new[] { "10", "11" }, deletedIds); } + + [Fact] + public async Task CleanupValidation_RetainsStagedValueForRetry_WhenDeleteFails() + { + // The staged value must only be removed from tracking once DeleteRecordAsync actually + // succeeds -- popping it up front would lose it on a failed attempt, breaking a retry. + var recordsCallCount = 0; + var deletePaths = new List(); + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Post) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.Created, + "{\"domain_record\":{\"id\":1,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"ignored\",\"ttl\":300}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + recordsCallCount++; + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[{\"id\":10,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"value-A\",\"ttl\":300}]}"); + } + + if (req.Method == HttpMethod.Delete) + { + deletePaths.Add(req.RequestUri.PathAndQuery); + if (recordsCallCount == 1) + { + return new HttpResponseMessage(HttpStatusCode.InternalServerError) + { + Content = new StringContent("{\"message\":\"boom\"}") + }; + } + return new HttpResponseMessage(HttpStatusCode.NoContent); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + const string key = "_acme-challenge.example.com"; + var stage = await validator.StageValidation(key, "value-A", CancellationToken.None); + Assert.True(stage.Success); + + var firstCleanup = await validator.CleanupValidation(key, CancellationToken.None); + Assert.False(firstCleanup.Success); + + var secondCleanup = await validator.CleanupValidation(key, CancellationToken.None); + Assert.True(secondCleanup.Success); + + Assert.Equal(2, deletePaths.Count); + Assert.All(deletePaths, p => Assert.EndsWith("/10", p)); + } + + [Fact] + public async Task StageValidation_SanitizesKeyInErrorMessageAndLog() + { + var handler = new FakeHttpMessageHandler(_ => + FakeHttpMessageHandler.Json(HttpStatusCode.Unauthorized, "{\"message\":\"nope\"}")); + + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + var result = await validator.StageValidation( + "_acme-challenge.example.com\r\nFORGED LOG LINE", "value", CancellationToken.None); + + Assert.False(result.Success); + Assert.DoesNotContain("\r", result.ErrorMessage); + Assert.DoesNotContain("\n", result.ErrorMessage); + } } } diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs index 7176466..f21e898 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs @@ -94,16 +94,16 @@ public async Task StageValidation(string key, string val return new DomainValidationResult { Success = success, - ErrorMessage = success ? null : $"Failed to create DNS {RecordTypeName} record for {key}" + ErrorMessage = success ? null : $"Failed to create DNS {RecordTypeName} record for {SafeForLog(key)}" }; } catch (Exception ex) { - _logger.LogError(ex, "DigitalOcean StageValidation failed for {RecordType} record '{Key}'", RecordTypeName, key); + _logger.LogError(ex, "DigitalOcean StageValidation failed for {RecordType} record '{Key}'", RecordTypeName, SafeForLog(key)); return new DomainValidationResult { Success = false, - ErrorMessage = $"Failed to create {RecordTypeName} record for {key}: {ex.Message}" + ErrorMessage = $"Failed to create {RecordTypeName} record for {SafeForLog(key)}: {ex.Message}" }; } } @@ -112,30 +112,66 @@ public async Task CleanupValidation(string key, Cancella { try { + // Peek (don't remove) the oldest still-pending staged value. It is only actually + // dequeued below once DeleteRecordAsync confirms success — removing it up front + // would permanently lose it if the delete failed/was retried, corrupting the queue + // for any later retry or sibling cleanup call on this key. string expectedValue = null; + int outstandingCount; lock (_stagedValuesLock) { if (_stagedValues.TryGetValue(key, out var queue) && queue.Count > 0) { - expectedValue = queue.Dequeue(); + expectedValue = queue.Peek(); + outstandingCount = queue.Count; + } + else + { + outstandingCount = 0; } } + if (outstandingCount > 1) + { + // CleanupValidation's own contract gives us no challenge value to match against + // (only `key`), so when more than one value is outstanding for the same key + // (e.g. an apex + wildcard SAN sharing one _acme-challenge FQDN) we cannot know + // FOR CERTAIN which one this specific cleanup call is for. We fall back to a + // best-effort FIFO match (oldest staged, oldest cleaned up) rather than refusing + // to clean up at all, but that assumption can be wrong if completion order + // doesn't match staging order -- surfacing it here so it's operationally visible + // rather than a silent, unverifiable guess. + _logger.LogWarning( + "{Count} {RecordType} values are still staged for '{Key}'; cleanup will match the oldest staged value on a best-effort basis, since CleanupValidation does not receive the specific challenge value", + outstandingCount, RecordTypeName, SafeForLog(key)); + } + var success = await _provider.DeleteRecordAsync(key, RecordTypeName, expectedValue, cancellationToken); + if (success && expectedValue != null) + { + lock (_stagedValuesLock) + { + if (_stagedValues.TryGetValue(key, out var queue) && queue.Count > 0 && queue.Peek() == expectedValue) + { + queue.Dequeue(); + } + } + } + return new DomainValidationResult { Success = success, - ErrorMessage = success ? null : $"Failed to delete DNS {RecordTypeName} record for {key}" + ErrorMessage = success ? null : $"Failed to delete DNS {RecordTypeName} record for {SafeForLog(key)}" }; } catch (Exception ex) { - _logger.LogError(ex, "DigitalOcean CleanupValidation failed for {RecordType} record '{Key}'", RecordTypeName, key); + _logger.LogError(ex, "DigitalOcean CleanupValidation failed for {RecordType} record '{Key}'", RecordTypeName, SafeForLog(key)); return new DomainValidationResult { Success = false, - ErrorMessage = $"Failed to delete {RecordTypeName} record for {key}: {ex.Message}" + ErrorMessage = $"Failed to delete {RecordTypeName} record for {SafeForLog(key)}: {ex.Message}" }; } } @@ -162,5 +198,12 @@ private string GetConfigValue(string key) } return string.Empty; } + + // DigitalOceanProvider sanitizes recordName before using it in ITS OWN log/exception + // messages, but that sanitized copy never crosses back into this class's `key` parameter + // (strings are immutable/passed by value) -- this class has its own independent log and + // ErrorMessage call sites that log/embed the raw `key`, so it needs its own sanitization + // pass to close the same CWE-117 CRLF log-forging gap at this layer. + private static string SafeForLog(string key) => DigitalOceanProvider.StripControlCharacters(key); } } diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs index fe028c8..da73835 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs @@ -184,8 +184,8 @@ public async Task DeleteRecordAsync(string recordName, string recordType, } _logger.LogInformation( - "Deleted {RecordType} record '{RelativeName}' in DigitalOcean zone '{Zone}'", - recordType, relativeName, zone); + "Deleted {RecordType} record '{RelativeName}' ({RecordId}) in DigitalOcean zone '{Zone}', matched by {MatchMode}", + recordType, relativeName, match.Id, zone, expectedValue != null ? "value" : "name"); return true; } @@ -204,8 +204,21 @@ private async Task FindZoneForRecordAsync(string recordName, Cancellatio var zoneNames = new List(); var nextUri = "domains?per_page=200"; + // Bounds the loop against a malformed/cyclic `next` link (API bug or future change) so + // a broken pagination response can't hang StageValidation/CleanupValidation forever. + // 5,000 pages at 200/page is 1,000,000 domains -- far beyond any realistic account size. + const int maxPages = 5000; + var pageCount = 0; + while (!string.IsNullOrEmpty(nextUri)) { + pageCount++; + if (pageCount > maxPages) + { + throw new InvalidOperationException( + $"DigitalOcean domain list pagination exceeded {maxPages} pages without terminating; aborting."); + } + // `next` (when present) is an ABSOLUTE URL from DigitalOcean's HATEOAS-style // pagination, e.g. "https://api.digitalocean.com/v2/domains?page=2&per_page=200". // HttpClient.GetAsync uses an absolute URI as-is, ignoring BaseAddress, so passing From 6d8c9c30d315c535f623abb4902750778f82d2fe Mon Sep 17 00:00:00 2001 From: Sean <1661003+spbsoluble@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:18:51 -0700 Subject: [PATCH 5/8] fix: serialize per-key cleanup, log created record id, sanitize vendor response bodies Round 3 full-review findings on the round-2 fix commit: - The per-key lock introduced in round 2 only covered the queue peek and the final conditional dequeue, not the DeleteRecordAsync call between them. Two concurrent CleanupValidation calls for the same key could both peek the same staged value before either dequeued it, letting one call's delete mask the loss of the other's distinct, still-existing record while both reported success. Now serializes ALL Stage/Cleanup calls for a given key end-to-end (including the network round-trip) via a per-key SemaphoreSlim; different keys still run fully in parallel. - CreateRecordAsync never logged the newly created record's ID (the response type for it existed but was unused), so the audit trail couldn't correlate which DigitalOcean record came from which staging call when disambiguating a later best-effort cleanup match. Now deserializes and logs it. - HTTP response bodies read from DigitalOcean (domain list, records list, create/delete failure bodies) were embedded into log messages and exception text without the same control-character stripping applied to plugin-derived values, reopening the CWE-117 log-forging gap for server-supplied content. Now sanitized immediately after read. --- .../DigitalOceanDomainValidatorTests.cs | 73 ++++++++++++ .../DigitalOceanDomainValidator.cs | 104 ++++++++++++------ .../DigitalOceanProvider.cs | 18 ++- 3 files changed, 156 insertions(+), 39 deletions(-) diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs index 5521a2d..9939d3d 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs @@ -179,5 +179,78 @@ public async Task StageValidation_SanitizesKeyInErrorMessageAndLog() Assert.DoesNotContain("\r", result.ErrorMessage); Assert.DoesNotContain("\n", result.ErrorMessage); } + + [Fact] + public async Task CleanupValidation_SerializesConcurrentCallsForTheSameKey() + { + // Regression test: two concurrent CleanupValidation calls for the same key must be + // fully serialized (including the network round-trip), not just around the queue + // peek/dequeue -- otherwise both could peek the same staged value before either + // dequeues it, letting one call's delete silently mask the loss of the other's. + var deletedIds = new List(); + var recordsInFlight = 0; + var maxRecordsInFlight = 0; + var gate = new object(); + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Post) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.Created, + "{\"domain_record\":{\"id\":1,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"ignored\",\"ttl\":300}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + lock (gate) + { + recordsInFlight++; + maxRecordsInFlight = Math.Max(maxRecordsInFlight, recordsInFlight); + } + Thread.Sleep(50); // widens the window so an unserialized race would be observed + lock (gate) + { + recordsInFlight--; + } + + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[" + + "{\"id\":10,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"value-A\",\"ttl\":300}," + + "{\"id\":11,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"value-B\",\"ttl\":300}]}"); + } + + if (req.Method == HttpMethod.Delete) + { + lock (gate) + { + deletedIds.Add(req.RequestUri.PathAndQuery.Split('/').Last()); + } + return new HttpResponseMessage(HttpStatusCode.NoContent); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + const string key = "_acme-challenge.example.com"; + await validator.StageValidation(key, "value-A", CancellationToken.None); + await validator.StageValidation(key, "value-B", CancellationToken.None); + + var cleanup1 = validator.CleanupValidation(key, CancellationToken.None); + var cleanup2 = validator.CleanupValidation(key, CancellationToken.None); + var results = await Task.WhenAll(cleanup1, cleanup2); + + Assert.All(results, r => Assert.True(r.Success)); + Assert.Equal(1, maxRecordsInFlight); + Assert.Equal(new[] { "10", "11" }, deletedIds.OrderBy(x => x)); + } } } diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs index f21e898..05778c1 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs @@ -25,9 +25,21 @@ public class DigitalOceanDomainValidator : IDomainValidator // at the same _acme-challenge FQDN). CleanupValidation's own signature (key only, no value) // can't tell us which record to delete, so this queue records staging order per key on a // best-effort FIFO basis: cleanup for a key removes the oldest still-pending value staged - // for it. A lock guards concurrent Stage/Cleanup calls across SANs on the same instance. + // for it. private readonly Dictionary> _stagedValues = new(); - private readonly object _stagedValuesLock = new(); + + // Serializes ALL Stage/Cleanup calls for the SAME key end-to-end, including the network + // round-trip -- not just the queue peek/dequeue. A lock held only around the queue + // read/write (and released across the `await DeleteRecordAsync(...)` call) is not enough: + // two concurrent CleanupValidation calls for the same key could both peek the same head + // value before either dequeues it, so one call's delete succeeds while the other's + // redundant delete finds nothing (idempotent no-op) and ALSO reports success -- silently + // leaking the second call's real, distinct record and leaving a stale queue entry that + // nothing ever dequeues. Different keys still run fully in parallel; only same-key + // operations are serialized. Guarded by `_locksLock`, a separate, short-lived lock used + // only to get-or-create a key's semaphore -- never held across an await. + private readonly Dictionary _keyLocks = new(); + private readonly object _locksLock = new(); public DigitalOceanDomainValidator() { @@ -74,21 +86,15 @@ public void Initialize(IDomainValidatorConfigProvider configProvider) public async Task StageValidation(string key, string value, CancellationToken cancellationToken) { + var keyLock = GetKeyLock(key); + await keyLock.WaitAsync(cancellationToken); try { var success = await _provider.CreateRecordAsync(key, value, RecordTypeName, cancellationToken); if (success) { - lock (_stagedValuesLock) - { - if (!_stagedValues.TryGetValue(key, out var queue)) - { - queue = new Queue(); - _stagedValues[key] = queue; - } - queue.Enqueue(value); - } + GetQueue(key, createIfMissing: true).Enqueue(value); } return new DomainValidationResult @@ -106,30 +112,28 @@ public async Task StageValidation(string key, string val ErrorMessage = $"Failed to create {RecordTypeName} record for {SafeForLog(key)}: {ex.Message}" }; } + finally + { + keyLock.Release(); + } } public async Task CleanupValidation(string key, CancellationToken cancellationToken) { + var keyLock = GetKeyLock(key); + await keyLock.WaitAsync(cancellationToken); try { // Peek (don't remove) the oldest still-pending staged value. It is only actually // dequeued below once DeleteRecordAsync confirms success — removing it up front // would permanently lose it if the delete failed/was retried, corrupting the queue - // for any later retry or sibling cleanup call on this key. - string expectedValue = null; - int outstandingCount; - lock (_stagedValuesLock) - { - if (_stagedValues.TryGetValue(key, out var queue) && queue.Count > 0) - { - expectedValue = queue.Peek(); - outstandingCount = queue.Count; - } - else - { - outstandingCount = 0; - } - } + // for any later retry or sibling cleanup call on this key. Holding `keyLock` for + // the entire method (including the network round-trip) guarantees no other + // Stage/Cleanup call for this SAME key can observe or mutate the queue in between, + // so this peek-then-conditionally-dequeue is race-free for a given key. + var queue = GetQueue(key, createIfMissing: false); + var outstandingCount = queue?.Count ?? 0; + var expectedValue = outstandingCount > 0 ? queue.Peek() : null; if (outstandingCount > 1) { @@ -150,13 +154,7 @@ public async Task CleanupValidation(string key, Cancella if (success && expectedValue != null) { - lock (_stagedValuesLock) - { - if (_stagedValues.TryGetValue(key, out var queue) && queue.Count > 0 && queue.Peek() == expectedValue) - { - queue.Dequeue(); - } - } + queue.Dequeue(); } return new DomainValidationResult @@ -174,6 +172,10 @@ public async Task CleanupValidation(string key, Cancella ErrorMessage = $"Failed to delete {RecordTypeName} record for {SafeForLog(key)}: {ex.Message}" }; } + finally + { + keyLock.Release(); + } } public async Task ValidateConfiguration(Dictionary configuration) @@ -205,5 +207,41 @@ private string GetConfigValue(string key) // ErrorMessage call sites that log/embed the raw `key`, so it needs its own sanitization // pass to close the same CWE-117 CRLF log-forging gap at this layer. private static string SafeForLog(string key) => DigitalOceanProvider.StripControlCharacters(key); + + private SemaphoreSlim GetKeyLock(string key) + { + lock (_locksLock) + { + if (!_keyLocks.TryGetValue(key, out var keyLock)) + { + keyLock = new SemaphoreSlim(1, 1); + _keyLocks[key] = keyLock; + } + return keyLock; + } + } + + // Only ever called while holding that key's semaphore (see GetKeyLock), so the returned + // Queue is never accessed by more than one caller at a time and needs no further + // locking around Enqueue/Peek/Dequeue -- only the lookup/creation in the shared dictionary + // itself needs the brief `_locksLock` (reused here rather than adding a third lock, since + // it is already the lock guarding shared-dictionary structural changes for this class). + private Queue GetQueue(string key, bool createIfMissing) + { + lock (_locksLock) + { + if (_stagedValues.TryGetValue(key, out var queue)) + { + return queue; + } + if (!createIfMissing) + { + return null; + } + queue = new Queue(); + _stagedValues[key] = queue; + return queue; + } + } } } diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs index da73835..122ce19 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs @@ -107,7 +107,12 @@ public async Task CreateRecordAsync(string recordName, string value, strin var content = new StringContent(json, Encoding.UTF8, "application/json"); var response = await _httpClient.PostAsync($"domains/{zone}/records", content, cancellationToken); - var result = await response.Content.ReadAsStringAsync(cancellationToken); + // Sanitized immediately: this is DigitalOcean-controlled response content, not just + // plugin-derived values, but it still reaches log/exception sinks below, so it's + // subject to the same CWE-117 CRLF log-forging risk as any other logged value. Valid + // JSON never contains raw (unescaped) control characters, so this is a no-op on the + // success/deserialization path. + var result = StripControlCharacters(await response.Content.ReadAsStringAsync(cancellationToken)); if (!response.IsSuccessStatusCode) { @@ -119,9 +124,10 @@ public async Task CreateRecordAsync(string recordName, string value, strin $"DigitalOcean API returned {(int)response.StatusCode} ({response.StatusCode}) creating {recordType} record '{relativeName}' in zone '{zone}': {result}"); } + var createdId = JsonSerializer.Deserialize(result)?.DomainRecord?.Id; _logger.LogInformation( - "Created {RecordType} record '{RelativeName}' in DigitalOcean zone '{Zone}'", - recordType, relativeName, zone); + "Created {RecordType} record '{RelativeName}' ({RecordId}) in DigitalOcean zone '{Zone}'", + recordType, relativeName, createdId, zone); return true; } @@ -134,7 +140,7 @@ public async Task DeleteRecordAsync(string recordName, string recordType, var relativeName = RelativeRecordName(zone, recordName); var recordsResp = await _httpClient.GetAsync($"domains/{zone}/records?type={recordType}", cancellationToken); - var recordsBody = await recordsResp.Content.ReadAsStringAsync(cancellationToken); + var recordsBody = StripControlCharacters(await recordsResp.Content.ReadAsStringAsync(cancellationToken)); if (!recordsResp.IsSuccessStatusCode) { _logger.LogError( @@ -174,7 +180,7 @@ public async Task DeleteRecordAsync(string recordName, string recordType, if (!deleteResp.IsSuccessStatusCode) { - var deleteBody = await deleteResp.Content.ReadAsStringAsync(cancellationToken); + var deleteBody = StripControlCharacters(await deleteResp.Content.ReadAsStringAsync(cancellationToken)); _logger.LogError( "DigitalOcean API rejected deletion of {RecordType} record '{RelativeName}' ({RecordId}) in zone '{Zone}'. Status: {StatusCode}. Response: {Response}", recordType, relativeName, match.Id, zone, (int)deleteResp.StatusCode, deleteBody); @@ -227,7 +233,7 @@ private async Task FindZoneForRecordAsync(string recordName, Cancellatio // segment and, combined with BaseAddress already ending in "/v2/", doubles it into // "/v2/v2/...", which the real API 404s. var response = await _httpClient.GetAsync(nextUri, cancellationToken); - var body = await response.Content.ReadAsStringAsync(cancellationToken); + var body = StripControlCharacters(await response.Content.ReadAsStringAsync(cancellationToken)); if (!response.IsSuccessStatusCode) { From aecf7063446876e5b20d5016e638d00a3a51ab32 Mon Sep 17 00:00:00 2001 From: Sean <1661003+spbsoluble@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:29:13 -0700 Subject: [PATCH 6/8] fix: don't let cancellation while queued on the key lock escape uncaught Round 4 full-review finding on the round-3 fix commit: - keyLock.WaitAsync(cancellationToken) was called before the try block in both StageValidation and CleanupValidation. If the token fired while a caller was queued behind another same-key operation, the resulting OperationCanceledException propagated straight out of the method, bypassing the catch block and violating this class's documented contract that no exception may escape into the gateway. Moved the wait inside the try, guarded by a lockAcquired flag so the finally still only releases a lock that was actually taken. - Documented the pre-existing (accepted, low-severity) unbounded growth of _keyLocks/_stagedValues as a known characteristic rather than adding eviction logic under time pressure. - Fixed FakeHttpMessageHandler to run its responder via Task.Run instead of Task.FromResult: a synchronously-completed task let a "fire without awaiting" call in a test run fully inline on the calling thread, so two calls issued without an intervening await never actually overlapped -- masking whether concurrency-related fixes were exercised at all. Added a regression test proving cancellation while queued on the per-key lock now returns Success=false instead of throwing. --- .../DigitalOceanDomainValidatorTests.cs | 49 +++++++++++++++++++ .../FakeHttpMessageHandler.cs | 10 +++- .../DigitalOceanDomainValidator.cs | 36 ++++++++++++-- 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs index 9939d3d..383a26d 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs @@ -252,5 +252,54 @@ public async Task CleanupValidation_SerializesConcurrentCallsForTheSameKey() Assert.Equal(1, maxRecordsInFlight); Assert.Equal(new[] { "10", "11" }, deletedIds.OrderBy(x => x)); } + + [Fact] + public async Task CleanupValidation_ReturnsFailureInsteadOfThrowing_WhenCanceledWhileWaitingForKeyLock() + { + // Regression test: cancellation while queued behind another same-key operation must be + // caught and returned as Success=false, per this class's documented contract that no + // exception may escape into the gateway -- not just cancellation during the HTTP call. + var firstCallGate = new ManualResetEventSlim(false); + var releaseFirstCall = new ManualResetEventSlim(false); + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + firstCallGate.Set(); + releaseFirstCall.Wait(TimeSpan.FromSeconds(5)); + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, "{\"domain_records\":[]}"); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + const string key = "_acme-challenge.example.com"; + var firstCall = validator.CleanupValidation(key, CancellationToken.None); + + Assert.True(firstCallGate.Wait(TimeSpan.FromSeconds(5)), "first call did not reach the records lookup in time"); + + using var cts = new CancellationTokenSource(); + var secondCall = validator.CleanupValidation(key, cts.Token); + await Task.Delay(50); + cts.Cancel(); + + var secondResult = await secondCall; + Assert.False(secondResult.Success); + Assert.NotNull(secondResult.ErrorMessage); + + releaseFirstCall.Set(); + var firstResult = await firstCall; + Assert.True(firstResult.Success); + } } } diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs index 64fff62..adf5b1e 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/FakeHttpMessageHandler.cs @@ -24,7 +24,15 @@ protected override Task SendAsync(HttpRequestMessage reques // lets tests verify a caller's CancellationToken actually reaches the HTTP layer. cancellationToken.ThrowIfCancellationRequested(); Requests.Add(request); - return Task.FromResult(_responder(request)); + + // Run the responder on a background thread rather than returning an already-completed + // Task.FromResult(...). A synchronously-completed task lets `await` continue inline on + // the calling thread with no real suspension -- so a responder that blocks (simulating + // slow I/O) blocks the CALLER's thread too, and a "fire without awaiting" call in a test + // doesn't actually run concurrently with the rest of that test method. Task.Run gives + // tests genuine interleaving to exercise real concurrency (e.g. two calls contending for + // the same lock). + return Task.Run(() => _responder(request), cancellationToken); } public static HttpResponseMessage Json(HttpStatusCode status, string body) diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs index 05778c1..4afd704 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs @@ -38,6 +38,13 @@ public class DigitalOceanDomainValidator : IDomainValidator // nothing ever dequeues. Different keys still run fully in parallel; only same-key // operations are serialized. Guarded by `_locksLock`, a separate, short-lived lock used // only to get-or-create a key's semaphore -- never held across an await. + // + // Neither this dictionary nor `_stagedValues` ever evicts an entry, so both grow for the + // life of the validator instance, one entry per distinct FQDN ever staged/cleaned up. + // Accepted: per-entry cost is small, a validator instance's lifetime is bounded by the + // hosting gateway process (restarted periodically for patching), and `key` values are + // domain names from certificates this account is actually enrolling, not attacker-supplied + // input from an untrusted boundary. private readonly Dictionary _keyLocks = new(); private readonly object _locksLock = new(); @@ -87,9 +94,17 @@ public void Initialize(IDomainValidatorConfigProvider configProvider) public async Task StageValidation(string key, string value, CancellationToken cancellationToken) { var keyLock = GetKeyLock(key); - await keyLock.WaitAsync(cancellationToken); + var lockAcquired = false; try { + // Waiting for the lock (not just the work after it) must be inside this try block: + // if cancellationToken fires while queued behind another same-key operation, + // WaitAsync throws, and this class's documented contract is that NO exception may + // escape StageValidation -- it must always come back as a caught, logged + // Success=false result. + await keyLock.WaitAsync(cancellationToken); + lockAcquired = true; + var success = await _provider.CreateRecordAsync(key, value, RecordTypeName, cancellationToken); if (success) @@ -114,16 +129,26 @@ public async Task StageValidation(string key, string val } finally { - keyLock.Release(); + if (lockAcquired) + { + keyLock.Release(); + } } } public async Task CleanupValidation(string key, CancellationToken cancellationToken) { var keyLock = GetKeyLock(key); - await keyLock.WaitAsync(cancellationToken); + var lockAcquired = false; try { + // See the identical comment in StageValidation: waiting for the lock must be + // inside this try block so a cancellation while queued behind another same-key + // operation is caught and converted to a logged Success=false, not an escaping + // exception. + await keyLock.WaitAsync(cancellationToken); + lockAcquired = true; + // Peek (don't remove) the oldest still-pending staged value. It is only actually // dequeued below once DeleteRecordAsync confirms success — removing it up front // would permanently lose it if the delete failed/was retried, corrupting the queue @@ -174,7 +199,10 @@ public async Task CleanupValidation(string key, Cancella } finally { - keyLock.Release(); + if (lockAcquired) + { + keyLock.Release(); + } } } From 76425f273f766079407c7858e49515d36bad2589 Mon Sep 17 00:00:00 2001 From: Sean <1661003+spbsoluble@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:40:08 -0700 Subject: [PATCH 7/8] fix: null-key escape path, case-insensitive key tracking, bounded HTTP timeout Round 5 full-review findings (a convergence round): - GetKeyLock(key) was called before the try block in both StageValidation and CleanupValidation. A null key makes the underlying dictionary lookup throw ArgumentNullException, which would escape uncaught -- the same class of contract violation fixed for cancellation in round 4, one line earlier. Moved inside the try alongside the lock wait. - _stagedValues and _keyLocks used ordinal (case-sensitive) string keys while DigitalOceanProvider already matches DNS record names case-insensitively. A Stage/Cleanup pair for the same domain differing only in casing would silently fail to correlate, falling back to the pre-fix name-only match this branch's disambiguation work was built to avoid. Both dictionaries now use StringComparer.OrdinalIgnoreCase. - HttpClient never set an explicit Timeout, so under round 3's per-key lock (now held across the full network round-trip) a single stalled DigitalOcean connection could block an unrelated, legitimate operation for the same key for several minutes (the .NET default of 100s, times up to 3 sequential requests per Create/Delete call). Bounded to 30 seconds. Correctness and security lenses independently re-verified the round 1-4 fixes end-to-end this round with no further findings beyond these three; compliance lens converged clean. --- .../DigitalOceanDomainValidatorTests.cs | 86 +++++++++++++++++++ .../DigitalOceanDomainValidator.cs | 36 +++++--- .../DigitalOceanProvider.cs | 9 +- 3 files changed, 116 insertions(+), 15 deletions(-) diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs index 383a26d..a5085f8 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanDomainValidatorTests.cs @@ -301,5 +301,91 @@ public async Task CleanupValidation_ReturnsFailureInsteadOfThrowing_WhenCanceled var firstResult = await firstCall; Assert.True(firstResult.Success); } + + [Fact] + public async Task StageValidation_ReturnsFailureInsteadOfThrowing_WhenKeyIsNull() + { + // Regression test: GetKeyLock's dictionary lookup throws ArgumentNullException on a + // null key -- that call must be inside the try block so it comes back as a caught, + // logged Success=false result rather than an escaping exception. + var handler = new FakeHttpMessageHandler(_ => + throw new InvalidOperationException("Should not make HTTP calls")); + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + var result = await validator.StageValidation(null, "value", CancellationToken.None); + + Assert.False(result.Success); + Assert.NotNull(result.ErrorMessage); + } + + [Fact] + public async Task CleanupValidation_ReturnsFailureInsteadOfThrowing_WhenKeyIsNull() + { + var handler = new FakeHttpMessageHandler(_ => + throw new InvalidOperationException("Should not make HTTP calls")); + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + var result = await validator.CleanupValidation(null, CancellationToken.None); + + Assert.False(result.Success); + Assert.NotNull(result.ErrorMessage); + } + + [Fact] + public async Task StagedValueTracking_CorrelatesKeysCaseInsensitively() + { + // DNS names are inherently case-insensitive, and DigitalOceanProvider already matches + // record names with OrdinalIgnoreCase -- the staged-value tracking used to disambiguate + // cleanup must not silently fail to correlate a Stage/Cleanup pair that differ only in + // casing. Two same-name records are returned in the OPPOSITE order from staging, so a + // name-only fallback (what would happen if the case difference broke correlation and + // lost the staged value) would delete the wrong one. + HttpRequestMessage deleteRequest = null; + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Post) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.Created, + "{\"domain_record\":{\"id\":1,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"ignored\",\"ttl\":300}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/records")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[" + + "{\"id\":11,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"value-B\",\"ttl\":300}," + + "{\"id\":10,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"value-A\",\"ttl\":300}]}"); + } + + if (req.Method == HttpMethod.Delete) + { + deleteRequest = req; + return new HttpResponseMessage(HttpStatusCode.NoContent); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + var validator = new DigitalOceanDomainValidator(provider); + + var stage = await validator.StageValidation("_acme-challenge.Example.com", "value-A", CancellationToken.None); + Assert.True(stage.Success); + + var cleanup = await validator.CleanupValidation("_acme-challenge.example.com", CancellationToken.None); + + Assert.True(cleanup.Success); + Assert.NotNull(deleteRequest); + Assert.EndsWith("domains/example.com/records/10", deleteRequest.RequestUri.PathAndQuery); + } } } diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs index 4afd704..06b79ce 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanDomainValidator.cs @@ -25,8 +25,11 @@ public class DigitalOceanDomainValidator : IDomainValidator // at the same _acme-challenge FQDN). CleanupValidation's own signature (key only, no value) // can't tell us which record to delete, so this queue records staging order per key on a // best-effort FIFO basis: cleanup for a key removes the oldest still-pending value staged - // for it. - private readonly Dictionary> _stagedValues = new(); + // for it. Keyed case-insensitively: DNS names are inherently case-insensitive, and + // DigitalOceanProvider already matches record names with StringComparison.OrdinalIgnoreCase + // -- an ordinal-cased dictionary here could silently fail to correlate a Stage/Cleanup pair + // that differ only in casing, defeating this exact disambiguation mechanism. + private readonly Dictionary> _stagedValues = new(StringComparer.OrdinalIgnoreCase); // Serializes ALL Stage/Cleanup calls for the SAME key end-to-end, including the network // round-trip -- not just the queue peek/dequeue. A lock held only around the queue @@ -45,7 +48,8 @@ public class DigitalOceanDomainValidator : IDomainValidator // hosting gateway process (restarted periodically for patching), and `key` values are // domain names from certificates this account is actually enrolling, not attacker-supplied // input from an untrusted boundary. - private readonly Dictionary _keyLocks = new(); + // Same case-insensitivity rationale as `_stagedValues` above. + private readonly Dictionary _keyLocks = new(StringComparer.OrdinalIgnoreCase); private readonly object _locksLock = new(); public DigitalOceanDomainValidator() @@ -93,15 +97,18 @@ public void Initialize(IDomainValidatorConfigProvider configProvider) public async Task StageValidation(string key, string value, CancellationToken cancellationToken) { - var keyLock = GetKeyLock(key); + SemaphoreSlim keyLock = null; var lockAcquired = false; try { - // Waiting for the lock (not just the work after it) must be inside this try block: - // if cancellationToken fires while queued behind another same-key operation, - // WaitAsync throws, and this class's documented contract is that NO exception may - // escape StageValidation -- it must always come back as a caught, logged - // Success=false result. + // GetKeyLock (a Dictionary lookup) and the lock wait itself must both be inside + // this try block: a null key would make GetKeyLock's TryGetValue throw + // ArgumentNullException, and a cancellation while queued behind another same-key + // operation makes WaitAsync throw -- either way, this class's documented contract + // is that NO exception may escape StageValidation, only a caught, logged + // Success=false result. `lockAcquired` staying false in either case correctly + // tells the finally below there is nothing to release. + keyLock = GetKeyLock(key); await keyLock.WaitAsync(cancellationToken); lockAcquired = true; @@ -138,14 +145,15 @@ public async Task StageValidation(string key, string val public async Task CleanupValidation(string key, CancellationToken cancellationToken) { - var keyLock = GetKeyLock(key); + SemaphoreSlim keyLock = null; var lockAcquired = false; try { - // See the identical comment in StageValidation: waiting for the lock must be - // inside this try block so a cancellation while queued behind another same-key - // operation is caught and converted to a logged Success=false, not an escaping - // exception. + // See the identical comment in StageValidation: GetKeyLock and the lock wait must + // both be inside this try block so a null key or a cancellation while queued behind + // another same-key operation is caught and converted to a logged Success=false, + // not an escaping exception. + keyLock = GetKeyLock(key); await keyLock.WaitAsync(cancellationToken); lockAcquired = true; diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs index 122ce19..ed5885f 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs @@ -87,7 +87,14 @@ internal DigitalOceanProvider(string apiToken, HttpMessageHandler handler) _httpClient = new HttpClient(handler) { - BaseAddress = new Uri("https://api.digitalocean.com/v2/") + BaseAddress = new Uri("https://api.digitalocean.com/v2/"), + // DigitalOceanDomainValidator holds a per-key lock across the entire Stage/Cleanup + // call, including every HTTP request this class makes -- without an explicit bound, + // a single stalled DigitalOcean connection falls back to HttpClient's 100-second + // default, and a Create/Delete can make 2-3 sequential requests, so an unrelated + // legitimate operation for the SAME key could queue behind a hung one for several + // minutes. 30 seconds is generous for a DNS record CRUD call under normal conditions. + Timeout = TimeSpan.FromSeconds(30) }; _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", apiToken); From 2f1d3002e24d416c714ec848b1e8bfc448d77ce7 Mon Sep 17 00:00:00 2001 From: Sean <1661003+spbsoluble@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:56:22 -0700 Subject: [PATCH 8/8] fix: paginate the records list used by DeleteRecordAsync Round 6 full-review finding (a convergence round): the domains list in FindZoneForRecordAsync was hardened with pagination across all five prior rounds, but the structurally identical domains/{zone}/records listing used by DeleteRecordAsync had none -- RecordsResponse didn't even declare a Links field to read a cursor from. A zone with more than one page of TXT records (other TXT records, concurrent SAN challenges, or previously stranded records) would have a target record on page 2+ silently treated as "not found", reporting cleanup as complete without deleting it and leaving it in DNS indefinitely -- no adversary required, just a normal zone with enough TXT records. Extracted the page cap (now MaxPaginationPages, shared with the domains listing) and added the same paging loop to the records fetch. Correctness and compliance lenses converged clean this round with no other findings. --- .../DigitalOceanProviderTests.cs | 50 ++++++++++++++++ .../DigitalOceanProvider.cs | 60 +++++++++++++------ 2 files changed, 93 insertions(+), 17 deletions(-) diff --git a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs index 7c0e2f8..dfa54c9 100644 --- a/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs +++ b/Keyfactor.DnsProvider.DigitalOcean.Tests/DigitalOceanProviderTests.cs @@ -229,6 +229,56 @@ public async Task DeleteRecordAsync_DeletesMatchingRecord() Assert.EndsWith("domains/example.com/records/7", deleteRequest.RequestUri.PathAndQuery); } + [Fact] + public async Task DeleteRecordAsync_FollowsPaginationToFindRecordOnLaterPage() + { + // A zone can have more than one page of records (other TXT records, concurrent SAN + // challenges, etc.) -- the target record living on page 2+ must not be treated as + // "not found" just because the first page didn't contain it. + var recordsRequests = new List(); + HttpRequestMessage deleteRequest = null; + + var handler = new FakeHttpMessageHandler(req => + { + if (req.Method == HttpMethod.Get && req.RequestUri.PathAndQuery.Contains("/domains?")) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domains\":[{\"name\":\"example.com\"}],\"links\":{}}"); + } + + if (req.Method == HttpMethod.Get && req.RequestUri.AbsolutePath.Equals("/v2/domains/example.com/records", StringComparison.OrdinalIgnoreCase)) + { + recordsRequests.Add(req); + if (recordsRequests.Count == 1) + { + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[{\"id\":1,\"type\":\"TXT\",\"name\":\"unrelated\",\"data\":\"x\",\"ttl\":300}]," + + "\"links\":{\"pages\":{\"next\":\"https://api.digitalocean.com/v2/domains/example.com/records?type=TXT&page=2&per_page=200\"}}}"); + } + + return FakeHttpMessageHandler.Json(HttpStatusCode.OK, + "{\"domain_records\":[{\"id\":7,\"type\":\"TXT\",\"name\":\"_acme-challenge\",\"data\":\"abc123\",\"ttl\":300}]}"); + } + + if (req.Method == HttpMethod.Delete) + { + deleteRequest = req; + return new HttpResponseMessage(HttpStatusCode.NoContent); + } + + throw new InvalidOperationException($"Unexpected request: {req.Method} {req.RequestUri}"); + }); + + var provider = new DigitalOceanProvider("token", handler); + + var result = await provider.DeleteRecordAsync("_acme-challenge.example.com", "TXT"); + + Assert.True(result); + Assert.Equal(2, recordsRequests.Count); + Assert.NotNull(deleteRequest); + Assert.EndsWith("domains/example.com/records/7", deleteRequest.RequestUri.PathAndQuery); + } + [Fact] public async Task DeleteRecordAsync_IsIdempotentWhenRecordAlreadyGone() { diff --git a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs index ed5885f..fdad29e 100644 --- a/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs +++ b/Keyfactor.DnsProvider.DigitalOcean/DigitalOceanProvider.cs @@ -13,6 +13,12 @@ internal class DigitalOceanProvider { private static readonly ILogger _logger = LogHandler.GetClassLogger(); + // Bounds any DigitalOcean list-endpoint pagination loop against a malformed/cyclic `next` + // link (API bug or future change) so a broken pagination response can't hang + // StageValidation/CleanupValidation forever. 5,000 pages at 200/page is 1,000,000 items -- + // far beyond any realistic account's domain or per-zone record count. + private const int MaxPaginationPages = 5000; + private readonly HttpClient _httpClient; private class DomainData @@ -64,6 +70,9 @@ private class RecordsResponse { [JsonPropertyName("domain_records")] public RecordData[] DomainRecords { get; set; } + + [JsonPropertyName("links")] + public LinksData Links { get; set; } } private class CreateRecordResponse @@ -146,19 +155,41 @@ public async Task DeleteRecordAsync(string recordName, string recordType, var zone = await FindZoneForRecordAsync(recordName, cancellationToken); var relativeName = RelativeRecordName(zone, recordName); - var recordsResp = await _httpClient.GetAsync($"domains/{zone}/records?type={recordType}", cancellationToken); - var recordsBody = StripControlCharacters(await recordsResp.Content.ReadAsStringAsync(cancellationToken)); - if (!recordsResp.IsSuccessStatusCode) + // Paged the same way as FindZoneForRecordAsync's domain listing: a zone can accumulate + // more than one page of records (other TXT records, multiple concurrent SAN + // challenges, or previously-stranded records), and an un-paginated fetch would make + // records on page 2+ invisible to the match below -- silently treating a genuinely + // still-live record as "not found" and reporting cleanup as complete without deleting + // it, leaving it in DNS indefinitely. + var records = new List(); + var nextRecordsUri = $"domains/{zone}/records?type={recordType}&per_page=200"; + var recordsPageCount = 0; + + while (!string.IsNullOrEmpty(nextRecordsUri)) { - _logger.LogError( - "DigitalOcean API failed to list records for zone '{Zone}'. Status: {StatusCode}. Response: {Response}", - zone, (int)recordsResp.StatusCode, recordsBody); + recordsPageCount++; + if (recordsPageCount > MaxPaginationPages) + { + throw new InvalidOperationException( + $"DigitalOcean record list pagination for zone '{zone}' exceeded {MaxPaginationPages} pages without terminating; aborting."); + } - throw new InvalidOperationException( - $"DigitalOcean API returned {(int)recordsResp.StatusCode} ({recordsResp.StatusCode}) listing records in zone '{zone}': {recordsBody}"); - } + var recordsResp = await _httpClient.GetAsync(nextRecordsUri, cancellationToken); + var recordsBody = StripControlCharacters(await recordsResp.Content.ReadAsStringAsync(cancellationToken)); + if (!recordsResp.IsSuccessStatusCode) + { + _logger.LogError( + "DigitalOcean API failed to list records for zone '{Zone}'. Status: {StatusCode}. Response: {Response}", + zone, (int)recordsResp.StatusCode, recordsBody); + + throw new InvalidOperationException( + $"DigitalOcean API returned {(int)recordsResp.StatusCode} ({recordsResp.StatusCode}) listing records in zone '{zone}': {recordsBody}"); + } - var records = JsonSerializer.Deserialize(recordsBody)?.DomainRecords ?? Array.Empty(); + var recordsPage = JsonSerializer.Deserialize(recordsBody); + records.AddRange(recordsPage?.DomainRecords ?? Array.Empty()); + nextRecordsUri = recordsPage?.Links?.Pages?.Next; + } // When multiple records share the same name/type (e.g. an apex + wildcard SAN both // challenging at the same _acme-challenge FQDN with different values), matching by @@ -216,20 +247,15 @@ private async Task FindZoneForRecordAsync(string recordName, Cancellatio var zoneNames = new List(); var nextUri = "domains?per_page=200"; - - // Bounds the loop against a malformed/cyclic `next` link (API bug or future change) so - // a broken pagination response can't hang StageValidation/CleanupValidation forever. - // 5,000 pages at 200/page is 1,000,000 domains -- far beyond any realistic account size. - const int maxPages = 5000; var pageCount = 0; while (!string.IsNullOrEmpty(nextUri)) { pageCount++; - if (pageCount > maxPages) + if (pageCount > MaxPaginationPages) { throw new InvalidOperationException( - $"DigitalOcean domain list pagination exceeded {maxPages} pages without terminating; aborting."); + $"DigitalOcean domain list pagination exceeded {MaxPaginationPages} pages without terminating; aborting."); } // `next` (when present) is an ABSOLUTE URL from DigitalOcean's HATEOAS-style