diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs index 27e64bb6..859ce505 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs @@ -7,6 +7,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts; using ZB.MOM.WW.ScadaBridge.Commons.Types; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Secrets; using ZB.MOM.WW.ScadaBridge.Communication; namespace ZB.MOM.WW.ScadaBridge.DeploymentManager; @@ -40,6 +41,7 @@ public class ArtifactDeploymentService private readonly DeploymentManagerOptions _options; private readonly ILogger _logger; private readonly ArtifactRevisionHashService _hashService; + private readonly SiteSecretsCollector _secretsCollector; /// /// Initializes a new instance of the ArtifactDeploymentService. @@ -60,8 +62,13 @@ public class ArtifactDeploymentService /// Logger instance. /// /// Computes the deterministic revision hash of a per-site artifact set, stamped on the - /// row after a successful deploy. Last on the signature so the - /// addition stays positionally additive for existing call sites. + /// row after a successful deploy. Second-to-last on the signature so + /// the addition stayed positionally additive for existing call sites at the time it was + /// introduced. + /// + /// + /// Resolves a site's referenced secrets (ciphertext rows) against the central secret + /// store (design §3.1/§3.3). Last on the signature for the same reason. /// public ArtifactDeploymentService( ISiteRepository siteRepo, @@ -73,7 +80,8 @@ public class ArtifactDeploymentService IAuditService auditService, IOptions options, ILogger logger, - ArtifactRevisionHashService hashService) + ArtifactRevisionHashService hashService, + SiteSecretsCollector secretsCollector) { _siteRepo = siteRepo; _deploymentRepo = deploymentRepo; @@ -87,6 +95,7 @@ public class ArtifactDeploymentService _options = options.Value; _logger = logger; _hashService = hashService; + _secretsCollector = secretsCollector; } /// @@ -101,7 +110,12 @@ public class ArtifactDeploymentService /// UI summary, and persisted record correlate. When null a fresh id is minted (used by /// single-site retries). /// - /// A deployment artifacts command for the site. + /// + /// The deployment artifacts command for the site, and any secret references its data + /// connections make that could not be resolved at central (absent or tombstoned). A + /// non-empty Missing list means the command must NOT be sent to the site — + /// see /. + /// /// /// This convenience overload runs the global artifact queries /// for a single site (used by ). The multi-site @@ -109,7 +123,7 @@ public class ArtifactDeploymentService /// per-site loop and calls the prefetched-globals overload to avoid the N+1 /// re-query of every system-wide artifact set per site. /// - public async Task BuildDeployArtifactsCommandAsync( + public async Task<(DeployArtifactsCommand Command, IReadOnlyList Missing)> BuildDeployArtifactsCommandAsync( int siteId, CancellationToken cancellationToken = default, string? deploymentId = null) @@ -121,7 +135,8 @@ public class ArtifactDeploymentService /// /// Builds a per-site using a previously-fetched /// snapshot of the global artifact sets (shared scripts, external systems + methods, - /// DB connections). Only the per-site data-connection query runs here. + /// DB connections). Only the per-site data-connection query — and the per-site secret + /// resolution it drives — runs here. /// /// /// Separating the global fetch from the per-site build lets @@ -134,8 +149,15 @@ public class ArtifactDeploymentService /// delivery is central-only and no notification artifact or SMTP credential is /// ever distributed to a site. The fields remain on the contract only for /// additive compatibility. + /// + /// The command's Secrets/MissingSecrets fields (design §3.1/§3.3) are + /// populated from regardless of whether + /// any reference is missing — MissingSecrets is CENTRAL-ONLY (never mapped to the + /// wire) and exists solely so can fold a + /// distinguishable marker into the hash even for a command the caller ultimately refuses + /// to send (see the returned Missing tuple element). /// - private async Task BuildDeployArtifactsCommandAsync( + private async Task<(DeployArtifactsCommand Command, IReadOnlyList Missing)> BuildDeployArtifactsCommandAsync( int siteId, GlobalArtifactSnapshot globals, CancellationToken cancellationToken, @@ -147,7 +169,9 @@ public class ArtifactDeploymentService var dataConnectionArtifacts = dataConnections.Select(dc => new DataConnectionArtifact(dc.Name, dc.Protocol, dc.PrimaryConfiguration, dc.BackupConfiguration, dc.FailoverRetryCount)).ToList(); - return new DeployArtifactsCommand( + var collected = await _secretsCollector.CollectAsync(dataConnectionArtifacts, cancellationToken); + + var command = new DeployArtifactsCommand( deploymentId ?? Guid.NewGuid().ToString("N"), globals.SharedScripts, globals.ExternalSystems, @@ -157,9 +181,23 @@ public class ArtifactDeploymentService dataConnectionArtifacts, // SMTP config (incl. credentials) is central-only — never shipped to sites. SmtpConfigurations: null, - DateTimeOffset.UtcNow); + DateTimeOffset.UtcNow, + Secrets: collected.Secrets.Count > 0 ? collected.Secrets : null, + MissingSecrets: collected.Missing.Count > 0 ? collected.Missing.Select(m => m.Name).ToList() : null); + + return (command, collected.Missing); } + /// + /// Formats the fail-closed message for one or more secret references a site's data + /// connections make that could not be resolved at central (design §3.3). Never includes + /// secret material — only names and the referencing connection names. + /// + private static string BuildMissingSecretsMessage(IReadOnlyList missing) => + string.Join("; ", missing.Select(m => + $"Secret '{m.Name}' referenced by data connection(s) {string.Join(", ", m.ReferencedBy)} " + + "is not in the central secret store")); + /// /// Fetches the system-wide artifact sets that are identical across every site — /// shared scripts, external systems (with their methods serialized in), and @@ -297,7 +335,11 @@ public class ArtifactDeploymentService // Every per-site command carries the SAME logical // deploymentId, so the per-site commands, audit log, persisted record, // and UI summary all reference one id instead of N+1 unrelated GUIDs. - var siteCommands = new Dictionary(); + // + // Each entry also carries the site's unresolved secret references (design §3.3): + // a non-empty Missing list means this site's command must never reach + // _communicationService — see the fan-out loop below. + var siteCommands = new Dictionary Missing)>(); foreach (var site in sites) { siteCommands[site.Id] = await BuildDeployArtifactsCommandAsync( @@ -327,7 +369,28 @@ public class ArtifactDeploymentService using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); cts.CancelAfter(_options.ArtifactDeploymentTimeoutPerSite); - var command = siteCommands[site.Id]; + var (command, missing) = siteCommands[site.Id]; + + // Fail this site closed BEFORE anything is sent (design §3.3): a referenced + // secret that is absent or tombstoned at central must not let the rest of the + // artifact set ship, or the site would read *Current* while a connection + // cannot resolve its key. Other sites in the batch are unaffected. + if (missing.Count > 0) + { + var missingMessage = BuildMissingSecretsMessage(missing); + _logger.LogError( + "Cannot deploy artifacts to site {SiteId} ({SiteName}): {Error}", + site.SiteIdentifier, site.Name, missingMessage); + return new SiteArtifactResult(site.SiteIdentifier, site.Name, false, missingMessage); + } + + if (command.Secrets is { Count: > 0 }) + { + _logger.LogInformation( + "Deploying {Count} secret(s) to site {SiteId}: {Secrets}", + command.Secrets.Count, site.SiteIdentifier, + string.Join(", ", command.Secrets.Select(s => $"{s.Name}@r{s.Revision}"))); + } _logger.LogInformation( "Deploying artifacts to site {SiteId} ({SiteName}), deploymentId={DeploymentId}", @@ -405,7 +468,7 @@ public class ArtifactDeploymentService if (successfulSites.Count > 0) { await StampArtifactRevisionAsync( - successfulSites.Select(site => (site, siteCommands[site.Id])).ToList(), + successfulSites.Select(site => (site, siteCommands[site.Id].Command)).ToList(), cancellationToken); } @@ -442,7 +505,26 @@ public class ArtifactDeploymentService using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); cts.CancelAfter(_options.ArtifactDeploymentTimeoutPerSite); - var command = await BuildDeployArtifactsCommandAsync(siteDbId, cts.Token); + var (command, missing) = await BuildDeployArtifactsCommandAsync(siteDbId, cts.Token); + + // Same fail-closed rule as the sweep path (design §3.3): a retry must not ship a + // site an artifact set whose secret references still don't resolve. + if (missing.Count > 0) + { + var missingMessage = BuildMissingSecretsMessage(missing); + _logger.LogError( + "Cannot deploy artifacts to site {SiteId}: {Error}", siteIdentifier, missingMessage); + return Result.Failure(missingMessage); + } + + if (command.Secrets is { Count: > 0 }) + { + _logger.LogInformation( + "Deploying {Count} secret(s) to site {SiteId}: {Secrets}", + command.Secrets.Count, siteIdentifier, + string.Join(", ", command.Secrets.Select(s => $"{s.Name}@r{s.Revision}"))); + } + var response = await _communicationService.DeployArtifactsAsync(siteIdentifier, command, cts.Token); var result = new SiteArtifactResult(siteIdentifier, siteIdentifier, response.Success, response.ErrorMessage); @@ -512,7 +594,11 @@ public class ArtifactDeploymentService var globals = await FetchGlobalArtifactsAsync(cancellationToken); foreach (var site in sites) { - var command = await BuildDeployArtifactsCommandAsync( + // The Missing tuple element is deliberately unused here: ComputeHash reads + // MissingSecrets straight off the command (design §3.4), so an unresolved + // reference still folds a distinguishable marker into the hash — the site reads + // *Out of date* instead of this call failing the whole batch. + var (command, _) = await BuildDeployArtifactsCommandAsync( site.Id, globals, cancellationToken, deploymentId: "current"); hashes[site.Id] = _hashService.ComputeHash(command); } diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ServiceCollectionExtensions.cs index 929e480e..fd649372 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ServiceCollectionExtensions.cs @@ -59,6 +59,17 @@ public static class ServiceCollectionExtensions services.AddScoped(); services.AddScoped(); + // Resolves a site's referenced secrets (ciphertext rows) against the central + // ISecretStore for the artifact deploy (design §3.1/§3.3). Singleton — it holds no + // state and the store it wraps is itself registered as a singleton + // (SecretsRegistration.AddScadaBridgeSecrets). Depends on ISecretStore, which the + // Central composition root always registers (Program.cs calls AddScadaBridgeSecrets + // before AddDeploymentManager's registrations are resolved) — a composition that + // builds ArtifactDeploymentService without ISecretStore registered is a wiring bug, + // not a supported configuration, so this deliberately does NOT fall back to a + // null/no-op store. + services.AddSingleton(); + // Deterministic hash of a per-site artifact set, stamped on the Site row after a // successful artifact deploy and recomputed to answer "are this site's artifacts // current?". Singleton — it holds no state and touches no DbContext. diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/SiteSecretsCollector.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/SiteSecretsCollector.cs new file mode 100644 index 00000000..5ab03826 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/SiteSecretsCollector.cs @@ -0,0 +1,68 @@ +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Secrets; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.ScadaBridge.DeploymentManager; + +/// +/// Result of : the resolved ciphertext rows for a +/// site's referenced secrets, and the references that could not be resolved (absent or +/// tombstoned) at central. +/// +public sealed record SiteSecretsCollection( + IReadOnlyList Secrets, + IReadOnlyList Missing); + +/// +/// Central half of design §3.1/§3.3: derives which secrets a site's data connections reference +/// (via ) and resolves each one against the central +/// , verbatim, as its ciphertext row. Never +/// decrypts and never sees plaintext — the store's already +/// returns only the encrypted representation. +/// +public sealed class SiteSecretsCollector(ISecretStore store) +{ + /// + /// Scans for secret:<name> references and + /// resolves each distinct name against the central secret store. + /// + /// The site's data-connection artifacts to scan. + /// A token to cancel the operation. + /// + /// The resolved rows (mapped field-for-field from ) and the + /// references that were absent or tombstoned in the store. + /// + public async Task CollectAsync( + IEnumerable connections, CancellationToken ct) + { + var secrets = new List(); + var missing = new List(); + + foreach (var reference in SecretReferenceScanner.Scan(connections)) + { + var row = await store.GetAsync(new SecretName(reference.Name), ct); + if (row is null || row.IsDeleted) + { + missing.Add(reference); + continue; + } + + secrets.Add(new SecretArtifact( + row.Name.Value, + row.ContentType.ToString(), + row.Ciphertext, + row.Nonce, + row.Tag, + row.WrappedDek, + row.WrapNonce, + row.WrapTag, + row.KekId, + row.Revision, + row.CreatedUtc, + row.UpdatedUtc, + row.Description)); + } + + return new SiteSecretsCollection(secrets, missing); + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ZB.MOM.WW.ScadaBridge.DeploymentManager.csproj b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ZB.MOM.WW.ScadaBridge.DeploymentManager.csproj index d86f3db7..c75024c4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ZB.MOM.WW.ScadaBridge.DeploymentManager.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ZB.MOM.WW.ScadaBridge.DeploymentManager.csproj @@ -12,6 +12,7 @@ + diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Admin/SitesPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Admin/SitesPageTests.cs index 68f17a21..2dc97d1d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Admin/SitesPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Admin/SitesPageTests.cs @@ -13,6 +13,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; using ZB.MOM.WW.ScadaBridge.Communication; using ZB.MOM.WW.ScadaBridge.DeploymentManager; using ZB.MOM.WW.ScadaBridge.DeploymentManager.PendingUpdates; +using ZB.MOM.WW.Secrets.Abstractions; using SitesPage = ZB.MOM.WW.ScadaBridge.CentralUI.Components.Pages.Admin.Sites; namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Admin; @@ -54,7 +55,10 @@ public class SitesPageTests : BunitContext Substitute.For(), Options.Create(new DeploymentManagerOptions()), NullLogger.Instance, - new ArtifactRevisionHashService()); + new ArtifactRevisionHashService(), + // These tests don't exercise secret references, so a store that never has + // anything is never called — CollectAsync only queries a scanned secret: reference. + new SiteSecretsCollector(Substitute.For())); Services.AddSingleton(artifactSvc); Services.AddSingleton(Substitute.For()); diff --git a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs index a4cffe83..ba37f60b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System.Text.Json; using Akka.Actor; using Akka.TestKit.Xunit2; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using NSubstitute; @@ -12,6 +13,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts; using ZB.MOM.WW.ScadaBridge.Communication; using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; +using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests; @@ -27,6 +29,7 @@ public class ArtifactDeploymentServiceTests : TestKit private readonly INotificationRepository _notificationRepo; private readonly IAuditService _audit; private readonly ArtifactRevisionHashService _hashService = new(); + private readonly ISecretStore _secretStore; public ArtifactDeploymentServiceTests() { @@ -39,8 +42,28 @@ public class ArtifactDeploymentServiceTests : TestKit // constructor dependency of ArtifactDeploymentService). _notificationRepo = Substitute.For(); _audit = Substitute.For(); + // Unconfigured GetAsync returns null (NSubstitute default), i.e. "not in the store" — + // exactly the fail-closed posture a test wants unless it explicitly seeds a row. + _secretStore = Substitute.For(); } + private static StoredSecret SecretRow(string name, long revision, string? description = null) => new() + { + Name = new SecretName(name), + Description = description, + ContentType = SecretContentType.Text, + Ciphertext = [1, 2, 3], + Nonce = [4, 5], + Tag = [6, 7], + WrappedDek = [8, 9], + WrapNonce = [10, 11], + WrapTag = [12, 13], + KekId = "kek-1", + Revision = revision, + CreatedUtc = DateTimeOffset.Parse("2026-01-01T00:00:00Z"), + UpdatedUtc = DateTimeOffset.Parse("2026-01-02T00:00:00Z"), + }; + [Fact] public async Task DeployToAllSitesAsync_NoSites_ReturnsFailure() { @@ -479,7 +502,126 @@ public class ArtifactDeploymentServiceTests : TestKit await _templateRepo.DidNotReceive().GetAllSharedScriptsAsync(Arg.Any()); } - private ArtifactDeploymentService CreateService() + // ── Secrets ride the artifact deploy (design §3.1/§3.3) ── + + [Fact] + public async Task DeployToAllSites_SiteWithMissingSecret_FailsThatSiteOnly_AndSendsNothingToIt() + { + var sites = new List + { + new("Site One", "ok-site") { Id = 1 }, + new("Site Two", "bad-site") { Id = 2 } + }; + _siteRepo.GetAllSitesAsync(Arg.Any()).Returns(sites); + _siteRepo.GetDataConnectionsBySiteIdAsync(1, Arg.Any()) + .Returns(new List()); + _siteRepo.GetDataConnectionsBySiteIdAsync(2, Arg.Any()) + .Returns(new List + { + new("GW1", "MxGateway", 2) { PrimaryConfiguration = """{"ApiKey":"secret:missing-key"}""" } + }); + // _secretStore is unconfigured for "missing-key" — GetAsync returns null (absent). + + var recorder = new ArtifactProbeRecorder(); + var probe = Sys.ActorOf(Props.Create(() => new ArtifactProbeActor(recorder))); + var service = CreateServiceWithCommActor(probe); + + var result = await service.DeployToAllSitesAsync("admin"); + + Assert.True(result.IsSuccess); + Assert.Equal(1, result.Value.SuccessCount); + Assert.Equal(1, result.Value.FailureCount); + Assert.Contains(result.Value.SiteResults, r => r.SiteId == "ok-site" && r.Success); + var badResult = Assert.Single(result.Value.SiteResults, r => r.SiteId == "bad-site"); + Assert.False(badResult.Success); + Assert.Equal( + "Secret 'missing-key' referenced by data connection(s) GW1 is not in the central secret store", + badResult.ErrorMessage); + + // Nothing was sent to the bad site: the probe only ever saw ok-site's command. + var sentCommand = Assert.Single(recorder.Received); + Assert.DoesNotContain( + sentCommand.DataConnections ?? [], + dc => dc.Name == "GW1"); + + // The bad site keeps no stamp — it still reads "out of date" / "never deployed". + var badSite = sites.Single(s => s.SiteIdentifier == "bad-site"); + Assert.Null(badSite.ArtifactRevisionHash); + } + + [Fact] + public async Task ComputeCurrentHashes_MissingSecret_UsesMissingMarker() + { + var site = new Site("Site One", "site-1") { Id = 1 }; + _siteRepo.GetDataConnectionsBySiteIdAsync(1, Arg.Any()) + .Returns(new List + { + new("GW1", "MxGateway", 1) { PrimaryConfiguration = """{"ApiKey":"secret:the-key"}""" } + }); + var service = CreateService(); + + // The reference does not resolve yet: ComputeCurrentArtifactHashesAsync must not + // fail the call — it folds a Missing marker into the hash instead. + var beforeCreated = await service.ComputeCurrentArtifactHashesAsync([site]); + + // The secret now exists in the store. + _secretStore.GetAsync(Arg.Is(n => n.Value == "the-key"), Arg.Any()) + .Returns(Task.FromResult(SecretRow("the-key", 1))); + var afterCreated = await service.ComputeCurrentArtifactHashesAsync([site]); + + Assert.NotEqual(beforeCreated[1], afterCreated[1]); + } + + [Fact] + public async Task DeployLogs_NeverContainSecretMaterial() + { + var ciphertext = new byte[] { 0xDE, 0xAD, 0xBE, 0xEF, 0x13, 0x37 }; + var resolvedRow = SecretRow("present-key", 5) with { Ciphertext = ciphertext }; + var ciphertextBase64 = Convert.ToBase64String(ciphertext); + var ciphertextHex = Convert.ToHexString(ciphertext); + + var sites = new List + { + new("Site One", "ok-site") { Id = 1 }, + new("Site Two", "bad-site") { Id = 2 } + }; + _siteRepo.GetAllSitesAsync(Arg.Any()).Returns(sites); + _siteRepo.GetDataConnectionsBySiteIdAsync(1, Arg.Any()) + .Returns(new List + { + new("GW1", "MxGateway", 1) { PrimaryConfiguration = """{"ApiKey":"secret:present-key"}""" } + }); + _siteRepo.GetDataConnectionsBySiteIdAsync(2, Arg.Any()) + .Returns(new List + { + new("GW2", "MxGateway", 2) { PrimaryConfiguration = """{"ApiKey":"secret:missing-key"}""" } + }); + _secretStore.GetAsync(Arg.Is(n => n.Value == "present-key"), Arg.Any()) + .Returns(Task.FromResult(resolvedRow)); + + var recorder = new ArtifactProbeRecorder(); + var probe = Sys.ActorOf(Props.Create(() => new ArtifactProbeActor(recorder))); + var capturingLogger = new CapturingLogger(); + var service = CreateServiceWithCommActor(probe, capturingLogger); + + var result = await service.DeployToAllSitesAsync("admin"); + + Assert.True(result.IsSuccess); + Assert.Equal(1, result.Value.SuccessCount); + Assert.Equal(1, result.Value.FailureCount); + + // The Information/Error lines still name what shipped and what didn't ... + Assert.Contains(capturingLogger.Messages, m => m.Contains("present-key@r5")); + Assert.Contains(capturingLogger.Messages, m => m.Contains("missing-key")); + Assert.Contains(capturingLogger.Messages, m => m.Contains("not in the central secret store")); + + // ... but never the ciphertext, in any rendering, in any log message or exception text. + Assert.DoesNotContain(capturingLogger.Messages, m => m.Contains(ciphertextBase64)); + Assert.DoesNotContain(capturingLogger.Messages, m => m.Contains(ciphertextHex)); + Assert.DoesNotContain(capturingLogger.Messages, m => m.Contains(ciphertextHex, StringComparison.OrdinalIgnoreCase)); + } + + private ArtifactDeploymentService CreateService(ILogger? logger = null) { var comms = new CommunicationService( Options.Create(new CommunicationOptions()), @@ -489,11 +631,13 @@ public class ArtifactDeploymentServiceTests : TestKit _siteRepo, _deploymentRepo, _templateRepo, _externalSystemRepo, _notificationRepo, comms, _audit, Options.Create(new DeploymentManagerOptions()), - NullLogger.Instance, - _hashService); + logger ?? NullLogger.Instance, + _hashService, + new SiteSecretsCollector(_secretStore)); } - private ArtifactDeploymentService CreateServiceWithCommActor(IActorRef commActor) + private ArtifactDeploymentService CreateServiceWithCommActor( + IActorRef commActor, ILogger? logger = null) { var comms = new CommunicationService( Options.Create(new CommunicationOptions @@ -510,8 +654,40 @@ public class ArtifactDeploymentServiceTests : TestKit { ArtifactDeploymentTimeoutPerSite = TimeSpan.FromSeconds(5) }), - NullLogger.Instance, - _hashService); + logger ?? NullLogger.Instance, + _hashService, + new SiteSecretsCollector(_secretStore)); + } + + /// + /// Captures every formatted log message (and, for entries logged with an exception, the + /// exception's ) so a test can assert on log CONTENT — + /// specifically, that secret material never appears in it. + /// + private sealed class CapturingLogger : ILogger + { + public List Messages { get; } = []; + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + { + Messages.Add(formatter(state, exception)); + if (exception is not null) + { + Messages.Add(exception.ToString()); + } + } + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + public void Dispose() { } + } } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/PendingUpdatesEvaluatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/PendingUpdatesEvaluatorTests.cs index 4becd79a..a76d9d02 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/PendingUpdatesEvaluatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/PendingUpdatesEvaluatorTests.cs @@ -16,6 +16,7 @@ using ZB.MOM.WW.ScadaBridge.Communication; using ZB.MOM.WW.ScadaBridge.DeploymentManager.PendingUpdates; using ZB.MOM.WW.ScadaBridge.TemplateEngine.Flattening; using ZB.MOM.WW.ScadaBridge.TemplateEngine.Validation; +using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests; @@ -66,7 +67,10 @@ public class PendingUpdatesEvaluatorTests comms, _audit, Options.Create(new DeploymentManagerOptions()), NullLogger.Instance, - _artifactHasher); + _artifactHasher, + // None of these tests reference secrets, so a store that never has anything is + // never exercised — CollectAsync only calls GetAsync for a scanned secret: reference. + new SiteSecretsCollector(Substitute.For())); _deploymentService = new DeploymentService( _deploymentRepo, diff --git a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/SiteSecretsCollectorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/SiteSecretsCollectorTests.cs new file mode 100644 index 00000000..5a832d6c --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/SiteSecretsCollectorTests.cs @@ -0,0 +1,136 @@ +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests; + +/// +/// is the central half of design §3.1/§3.3: derive which +/// secrets a site's data connections reference, resolve each against the central +/// , and report the ones that could not be resolved. It never sees +/// or produces plaintext — everything here is ciphertext bytes. +/// +public class SiteSecretsCollectorTests +{ + private static DataConnectionArtifact Dc(string name, string? primary, string? backup = null) + => new(name, "MxGateway", primary, backup, 3); + + private static StoredSecret Row(string name, long revision, bool deleted = false, string? description = null) => new() + { + Name = new SecretName(name), + Description = description, + ContentType = SecretContentType.Text, + Ciphertext = [1, 2, 3], + Nonce = [4, 5], + Tag = [6, 7], + WrappedDek = [8, 9], + WrapNonce = [10, 11], + WrapTag = [12, 13], + KekId = "kek-1", + Revision = revision, + IsDeleted = deleted, + CreatedUtc = DateTimeOffset.Parse("2026-01-01T00:00:00Z"), + UpdatedUtc = DateTimeOffset.Parse("2026-01-02T00:00:00Z"), + }; + + [Fact] + public async Task Collect_ReturnsOnlyReferencedRows_AndNamesMissing() + { + var store = new FakeSecretStore(Row("a", 3), Row("b", 1, deleted: true), Row("other", 1)); + + var result = await new SiteSecretsCollector(store).CollectAsync( + [Dc("GW1", """{"ApiKey":"secret:a"}"""), Dc("GW2", """{"ApiKey":"secret:b"}""")], default); + + // Only the referenced names come back — "other" is in the store but never referenced. + Assert.Equal(["a"], result.Secrets.Select(s => s.Name)); + Assert.Equal(3, result.Secrets[0].Revision); + // "b" is tombstoned (IsDeleted) — treated the same as absent. + Assert.Equal(["b"], result.Missing.Select(m => m.Name)); + Assert.Equal(["GW2"], result.Missing[0].ReferencedBy); + } + + [Fact] + public async Task Collect_ResolvedRow_MapsEveryFieldFromTheStoredSecretVerbatim() + { + var row = Row("mxgw-key", revision: 7, description: "MxGateway API key"); + var store = new FakeSecretStore(row); + + var result = await new SiteSecretsCollector(store).CollectAsync( + [Dc("GW1", """{"ApiKey":"secret:mxgw-key"}""")], default); + + var artifact = Assert.Single(result.Secrets); + Assert.Empty(result.Missing); + Assert.Equal(row.Name.Value, artifact.Name); + Assert.Equal(row.ContentType.ToString(), artifact.ContentType); + Assert.Equal(row.Ciphertext, artifact.Ciphertext); + Assert.Equal(row.Nonce, artifact.Nonce); + Assert.Equal(row.Tag, artifact.Tag); + Assert.Equal(row.WrappedDek, artifact.WrappedDek); + Assert.Equal(row.WrapNonce, artifact.WrapNonce); + Assert.Equal(row.WrapTag, artifact.WrapTag); + Assert.Equal(row.KekId, artifact.KekId); + Assert.Equal(row.Revision, artifact.Revision); + Assert.Equal(row.CreatedUtc, artifact.CreatedUtc); + Assert.Equal(row.UpdatedUtc, artifact.UpdatedUtc); + Assert.Equal(row.Description, artifact.Description); + } + + [Fact] + public async Task Collect_AbsentName_IsReportedMissing_NotThrown() + { + var store = new FakeSecretStore(); + + var result = await new SiteSecretsCollector(store).CollectAsync( + [Dc("GW1", """{"ApiKey":"secret:never-created"}""")], default); + + Assert.Empty(result.Secrets); + var missing = Assert.Single(result.Missing); + Assert.Equal("never-created", missing.Name); + Assert.Equal(["GW1"], missing.ReferencedBy); + } + + [Fact] + public async Task Collect_NoSecretReferences_ReturnsEmptyWithoutQueryingTheStore() + { + var store = new FakeSecretStore(Row("unused", 1)); + + var result = await new SiteSecretsCollector(store).CollectAsync( + [Dc("GW1", """{"Endpoint":"http://gw:5000"}""")], default); + + Assert.Empty(result.Secrets); + Assert.Empty(result.Missing); + } + + /// + /// In-memory stand-in. Implements only — + /// the sole member calls; every other member throws so an + /// accidental call (a write, a list, a replication apply) fails the test loudly instead of + /// silently no-op'ing. + /// + private sealed class FakeSecretStore : ISecretStore + { + private readonly Dictionary _rows; + + public FakeSecretStore(params StoredSecret[] rows) => _rows = rows.ToDictionary(r => r.Name); + + public Task GetAsync(SecretName name, CancellationToken ct) => + Task.FromResult(_rows.TryGetValue(name, out var row) ? row : null); + + public Task UpsertAsync(StoredSecret row, CancellationToken ct) => throw new NotSupportedException(); + + public Task DeleteAsync(SecretName name, string? actor, CancellationToken ct) => + throw new NotSupportedException(); + + public Task> ListAsync(bool includeDeleted, CancellationToken ct) => + throw new NotSupportedException(); + + public Task> GetManifestAsync(CancellationToken ct) => + throw new NotSupportedException(); + + public Task ApplyReplicatedAsync(StoredSecret row, CancellationToken ct) => + throw new NotSupportedException(); + + public Task ApplyRewrapAsync( + StoredSecret rewrappedRow, byte[] expectedCurrentWrappedDek, CancellationToken ct) => + throw new NotSupportedException(); + } +}