diff --git a/src/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager/Services/SsmSecretManagerService.cs b/src/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager/Services/SsmSecretManagerService.cs index 03e5cc6..4c7f51a 100644 --- a/src/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager/Services/SsmSecretManagerService.cs +++ b/src/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager/Services/SsmSecretManagerService.cs @@ -1,11 +1,13 @@ using Amazon.SimpleSystemsManagement; using Amazon.SimpleSystemsManagement.Model; using RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.Services.Resilience; +using RedShirt.Example.JobWorker.Common.SecretManagers.Core.Exceptions; using RedShirt.Example.JobWorker.Common.SecretManagers.Core.Services; +using System.Text.RegularExpressions; namespace RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.Services; -internal sealed class SsmSecretManagerService( +internal sealed partial class SsmSecretManagerService( IAmazonSimpleSystemsManagement ssm, ISsmRetryWrapperService retryWrapperService) : ISecretManagerService { @@ -14,8 +16,28 @@ internal sealed class SsmSecretManagerService( /// private const int MaxNamesPerRequest = 10; + /// + /// Regular expression for AWS Systems Manager Parameter Store hierarchical paths. + /// Paths must start with /, use only a-zA-Z0-9_.- in each segment, contain at most 15 + /// hierarchy levels, and be at most 2048 characters. + /// Source: https://docs.aws.amazon.com/systems-manager/latest/userguide/sysman-paramstore-su-create.html + /// + [GeneratedRegex(@"^(?=.{1,2048}$)(/[a-zA-Z0-9_.-]+){1,15}$")] + private static partial Regex ValidKeyRegex(); + + private static void ThrowIfInvalidKey(string key) + { + if (string.IsNullOrWhiteSpace(key) || !ValidKeyRegex().IsMatch(key)) + { + throw new WorkerSecretManagerException($"Invalid secret path: {key}") + {CouldBeTransient = false, IsHandled = false, CouldBeExternallySolvable = false}; + } + } + public async Task GetSecretAsync(string key, CancellationToken cancellationToken = default) { + ThrowIfInvalidKey(key); + var response = await retryWrapperService.RunAsync(ct => ssm.GetParameterAsync(new GetParameterRequest { @@ -29,6 +51,11 @@ public async Task GetSecretAsync(string key, CancellationToken cancellat public async Task> GetSecretsAsync(List keys, CancellationToken cancellationToken = default) { + foreach (var key in keys) + { + ThrowIfInvalidKey(key); + } + var result = new Dictionary(); var remaining = keys.Distinct().ToList(); diff --git a/src/RedShirt.Example.JobWorker.Common.Azure.KeyVaultSecretManager/Services/AzureKeyVaultService.cs b/src/RedShirt.Example.JobWorker.Common.Azure.KeyVaultSecretManager/Services/AzureKeyVaultService.cs index 743cb01..ea8b3e3 100644 --- a/src/RedShirt.Example.JobWorker.Common.Azure.KeyVaultSecretManager/Services/AzureKeyVaultService.cs +++ b/src/RedShirt.Example.JobWorker.Common.Azure.KeyVaultSecretManager/Services/AzureKeyVaultService.cs @@ -11,9 +11,13 @@ internal sealed partial class AzureKeyVaultService( IAzureRetryWrapperService retryWrapperService, IAzureKeyVaultClientSource clientSource) : ISecretManagerService { - private static bool IsValidKey(string key) + private static void ThrowIfInvalidKey(string key) { - return ValidKeyRegex().IsMatch(key); + if (string.IsNullOrWhiteSpace(key) || !ValidKeyRegex().IsMatch(key)) + { + throw new WorkerSecretManagerException($"Invalid secret path: {key}") + {CouldBeTransient = false, IsHandled = false, CouldBeExternallySolvable = false}; + } } /// @@ -28,11 +32,7 @@ private static bool IsValidKey(string key) public async Task GetSecretAsync(string key, CancellationToken cancellationToken = default) { - if (!IsValidKey(key)) - { - throw new WorkerSecretManagerException($"Invalid secret path: {key}") - {CouldBeTransient = false, IsHandled = false, CouldBeExternallySolvable = false}; - } + ThrowIfInvalidKey(key); try { @@ -57,10 +57,9 @@ public async Task GetSecretAsync(string key, CancellationToken cancellat public async Task> GetSecretsAsync(List keys, CancellationToken cancellationToken = default) { - if (keys.FirstOrDefault(key => !IsValidKey(key)) is { } badKey) + foreach (var key in keys) { - throw new WorkerSecretManagerException($"Invalid secret path: {badKey}") - {CouldBeTransient = false, IsHandled = false, CouldBeExternallySolvable = false}; + ThrowIfInvalidKey(key); } var items = new Dictionary(); diff --git a/test/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.UnitTests/Tests/Services/SsmSecretManagerServiceTests.cs b/test/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.UnitTests/Tests/Services/SsmSecretManagerServiceTests.cs index 56678f4..29518f3 100644 --- a/test/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.UnitTests/Tests/Services/SsmSecretManagerServiceTests.cs +++ b/test/RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.UnitTests/Tests/Services/SsmSecretManagerServiceTests.cs @@ -2,11 +2,19 @@ using Amazon.SimpleSystemsManagement.Model; using RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.Services; using RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.Services.Resilience; +using RedShirt.Example.JobWorker.Common.SecretManagers.Core.Exceptions; namespace RedShirt.Example.JobWorker.Common.Aws.SsmSecretManager.UnitTests.Tests.Services; public class SsmSecretManagerServiceTests { + private static string NewPath(string? prefix = null) + { + return string.IsNullOrEmpty(prefix) + ? $"/{Guid.NewGuid():N}" + : $"{prefix}/{Guid.NewGuid():N}"; + } + private sealed class PassthroughRetryWrapper : ISsmRetryWrapperService { public Task RunAsync(Func> func, CancellationToken cancellationToken = default) @@ -17,11 +25,52 @@ public Task RunAsync(Func> func, CancellationTo public class GetSecretAsync { + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("flat-name")] + [InlineData("bad key")] + [InlineData("/bad key")] + [InlineData("/bad$key")] + [InlineData("/trailing/")] + [InlineData("/empty//segment")] + [InlineData("/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p")] + public async Task InvalidKey_ThrowsSecretManagerExceptionWithoutCallingSsm(string key) + { + var ssm = new Mock(MockBehavior.Strict); + var service = new SsmSecretManagerService(ssm.Object, new PassthroughRetryWrapper()); + + var thrown = await Assert.ThrowsAsync(() => + service.GetSecretAsync(key, TestContext.Current.CancellationToken)); + + Assert.Equal($"Invalid secret path: {key}", thrown.Message); + Assert.False(thrown.CouldBeTransient); + Assert.False(thrown.IsHandled); + Assert.False(thrown.CouldBeExternallySolvable); + ssm.VerifyNoOtherCalls(); + } + + [Fact] + public async Task KeyLongerThan2048Characters_ThrowsSecretManagerException() + { + var key = "/" + new string('a', 2048); + var ssm = new Mock(MockBehavior.Strict); + var service = new SsmSecretManagerService(ssm.Object, new PassthroughRetryWrapper()); + + var thrown = await Assert.ThrowsAsync(() => + service.GetSecretAsync(key, TestContext.Current.CancellationToken)); + + Assert.Equal($"Invalid secret path: {key}", thrown.Message); + Assert.False(thrown.CouldBeTransient); + Assert.False(thrown.IsHandled); + ssm.VerifyNoOtherCalls(); + } + [Fact] public async Task PassesCancellationTokenToSsm() { using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); - var name = Guid.NewGuid().ToString("N"); + var name = NewPath(); var value = Guid.NewGuid().ToString("N"); var ssm = new Mock(MockBehavior.Strict); @@ -47,7 +96,7 @@ public async Task PassesCancellationTokenToSsm() [Fact] public async Task PropagatesSsmException() { - var name = Guid.NewGuid().ToString("N"); + var name = NewPath(); var ssm = new Mock(MockBehavior.Strict); ssm.Setup(s => s.GetParameterAsync(It.IsAny(), It.IsAny())) @@ -62,7 +111,7 @@ await Assert.ThrowsAsync(() => [Fact] public async Task ReturnsDecryptedParameterValue() { - var name = Guid.NewGuid().ToString("N"); + var name = NewPath(); var value = Guid.NewGuid().ToString("N"); var ssm = new Mock(MockBehavior.Strict); @@ -88,6 +137,36 @@ public async Task ReturnsDecryptedParameterValue() TestContext.Current.CancellationToken), Times.Once); ssm.VerifyNoOtherCalls(); } + + [Theory] + [InlineData("/a")] + [InlineData("/common/redis")] + [InlineData("/Dev/DBServer/MySQL/db-string13")] + [InlineData("/path.with-dots_and-dashes/value")] + [InlineData("/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o")] + public async Task ValidKey_ReturnsDecryptedParameterValue(string key) + { + var value = Guid.NewGuid().ToString("N"); + + var ssm = new Mock(MockBehavior.Strict); + ssm.Setup(s => s.GetParameterAsync( + It.Is(r => r.Name == key && r.WithDecryption == true), + It.IsAny())) + .ReturnsAsync(new GetParameterResponse + { + Parameter = new Parameter {Name = key, Value = value} + }); + + var service = new SsmSecretManagerService(ssm.Object, new PassthroughRetryWrapper()); + + var result = await service.GetSecretAsync(key, TestContext.Current.CancellationToken); + + Assert.Equal(value, result); + ssm.Verify(s => s.GetParameterAsync( + It.Is(r => r.Name == key && r.WithDecryption == true), + TestContext.Current.CancellationToken), Times.Once); + ssm.VerifyNoOtherCalls(); + } } public class GetSecretsAsync @@ -95,7 +174,7 @@ public class GetSecretsAsync [Fact] public async Task DeduplicatesNamesBeforeCallingSsm() { - var name = Guid.NewGuid().ToString("N"); + var name = NewPath(); var value = Guid.NewGuid().ToString("N"); var ssm = new Mock(MockBehavior.Strict); @@ -138,7 +217,7 @@ public async Task EmptyList_DoesNotCallSsm() [Fact] public async Task ExactlyTenNames_IsSingleRequest() { - var names = Enumerable.Range(0, 10).Select(i => $"/{i}/{Guid.NewGuid():N}").ToList(); + var names = Enumerable.Range(0, 10).Select(i => NewPath($"/{i}")).ToList(); var values = names.ToDictionary(n => n, _ => Guid.NewGuid().ToString("N")); var ssm = new Mock(MockBehavior.Strict); @@ -165,7 +244,7 @@ public async Task ExactlyTenNames_IsSingleRequest() [Fact] public async Task ExactlyTwentyNames_IsTwoFullBatches() { - var names = Enumerable.Range(0, 20).Select(i => $"/{i}/{Guid.NewGuid():N}").ToList(); + var names = Enumerable.Range(0, 20).Select(i => NewPath($"/{i}")).ToList(); var values = names.ToDictionary(n => n, _ => Guid.NewGuid().ToString("N")); var seenBatchSizes = new List(); @@ -193,10 +272,48 @@ public async Task ExactlyTwentyNames_IsTwoFullBatches() ssm.VerifyNoOtherCalls(); } + [Fact] + public async Task InvalidKey_ReportsFirstInvalidKeyInListOrder() + { + var ssm = new Mock(MockBehavior.Strict); + var service = new SsmSecretManagerService(ssm.Object, new PassthroughRetryWrapper()); + + var thrown = await Assert.ThrowsAsync(() => + service.GetSecretsAsync(["/ok-key", "bad key", "/also-bad!"], + TestContext.Current.CancellationToken)); + + Assert.Equal("Invalid secret path: bad key", thrown.Message); + Assert.False(thrown.CouldBeTransient); + Assert.False(thrown.IsHandled); + ssm.VerifyNoOtherCalls(); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("flat-name")] + [InlineData("bad key")] + [InlineData("/bad$key")] + public async Task InvalidKey_ThrowsBeforeCallingSsm(string badKey) + { + var validKey = NewPath(); + var ssm = new Mock(MockBehavior.Strict); + var service = new SsmSecretManagerService(ssm.Object, new PassthroughRetryWrapper()); + + var thrown = await Assert.ThrowsAsync(() => + service.GetSecretsAsync([validKey, badKey], TestContext.Current.CancellationToken)); + + Assert.Equal($"Invalid secret path: {badKey}", thrown.Message); + Assert.False(thrown.CouldBeTransient); + Assert.False(thrown.IsHandled); + Assert.False(thrown.CouldBeExternallySolvable); + ssm.VerifyNoOtherCalls(); + } + [Fact] public async Task MoreThanTenNames_IsChunkedAcrossRequests() { - var names = Enumerable.Range(0, 11).Select(i => $"/{i}/{Guid.NewGuid():N}").ToList(); + var names = Enumerable.Range(0, 11).Select(i => NewPath($"/{i}")).ToList(); var values = names.ToDictionary(n => n, _ => Guid.NewGuid().ToString("N")); var ssm = new Mock(MockBehavior.Strict); @@ -229,8 +346,8 @@ public async Task MoreThanTenNames_IsChunkedAcrossRequests() [Fact] public async Task OmitsInvalidParametersFromResult() { - var foundName = Guid.NewGuid().ToString("N"); - var missingName = Guid.NewGuid().ToString("N"); + var foundName = NewPath("/found"); + var missingName = NewPath("/missing"); var foundValue = Guid.NewGuid().ToString("N"); var ssm = new Mock(MockBehavior.Strict); @@ -265,7 +382,7 @@ public async Task OmitsInvalidParametersFromResult() public async Task PassesCancellationTokenToSsm() { using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); - var name = Guid.NewGuid().ToString("N"); + var name = NewPath(); var value = Guid.NewGuid().ToString("N"); var ssm = new Mock(MockBehavior.Strict); @@ -302,8 +419,8 @@ await Assert.ThrowsAsync(() => [Fact] public async Task ReturnsDecryptedParameterValues() { - var nameA = Guid.NewGuid().ToString("N"); - var nameB = Guid.NewGuid().ToString("N"); + var nameA = NewPath("/a"); + var nameB = NewPath("/b"); var valueA = Guid.NewGuid().ToString("N"); var valueB = Guid.NewGuid().ToString("N"); @@ -338,5 +455,33 @@ public async Task ReturnsDecryptedParameterValues() Times.Once); ssm.VerifyNoOtherCalls(); } + + [Theory] + [InlineData("/a")] + [InlineData("/common/redis")] + [InlineData("/Dev/DBServer/MySQL/db-string13")] + public async Task ValidKeys_ReturnDecryptedParameterValues(string key) + { + var value = Guid.NewGuid().ToString("N"); + + var ssm = new Mock(MockBehavior.Strict); + ssm.Setup(s => s.GetParametersAsync( + It.Is(r => + r.WithDecryption == true && r.Names.Count == 1 && r.Names[0] == key), + It.IsAny())) + .ReturnsAsync(new GetParametersResponse + { + Parameters = [new Parameter {Name = key, Value = value}] + }); + + var service = new SsmSecretManagerService(ssm.Object, new PassthroughRetryWrapper()); + + var result = await service.GetSecretsAsync([key], TestContext.Current.CancellationToken); + + Assert.Equal(new Dictionary {[key] = value}, result); + ssm.Verify(s => s.GetParametersAsync(It.IsAny(), It.IsAny()), + Times.Once); + ssm.VerifyNoOtherCalls(); + } } } \ No newline at end of file