From d4d1732a9e349eadf6a1c61bc49f5b9b0e6f13fa Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 10 Jul 2026 05:15:55 -0400 Subject: [PATCH] fix(management): elide DatabaseConnection ConnectionString from List/Get (arch-review C3) Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj --- .../Component-ManagementService.md | 2 + .../DatabaseConnectionDefCommands.cs | 5 +- .../ManagementActor.cs | 27 +++++++-- .../SecretProjectionTests.cs | 55 +++++++++++++++++++ 4 files changed, 83 insertions(+), 6 deletions(-) diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 3931eecc..59c3b572 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -190,6 +190,8 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B - **ListDatabaseConnections** / **GetDatabaseConnection**: Query database connection definitions. - **CreateDatabaseConnection** / **UpdateDatabaseConnection** / **DeleteDatabaseConnection**: Manage database connections. +> **Secret elision (arch-review C3).** The ADO.NET `ConnectionString` can embed the SQL password. List/Get responses are projected through `DatabaseConnectionPublicShape` (`{ id, name, hasConnectionString }`) — the connection string itself is never emitted. `UpdateDatabaseConnectionDefCommand.ConnectionString` is nullable and preserve-if-null: an omitted (`null`) value leaves the stored connection string intact, so a round-trip of the elided shape does not wipe the secret. + ### Inbound API Methods - **ListApiMethods** / **GetApiMethod**: Query inbound API method definitions. diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/DatabaseConnectionDefCommands.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/DatabaseConnectionDefCommands.cs index 16599a97..edfb3d59 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/DatabaseConnectionDefCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Management/DatabaseConnectionDefCommands.cs @@ -3,5 +3,8 @@ namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; public record ListDatabaseConnectionsCommand; public record GetDatabaseConnectionCommand(int DatabaseConnectionId); public record CreateDatabaseConnectionDefCommand(string Name, string ConnectionString); -public record UpdateDatabaseConnectionDefCommand(int DatabaseConnectionId, string Name, string ConnectionString); +// ConnectionString is nullable (arch-review C3): an omitted (null) value preserves +// the stored secret rather than wiping it — the connection string is elided from +// List/Get responses, so a round-trip must not require re-sending it. +public record UpdateDatabaseConnectionDefCommand(int DatabaseConnectionId, string Name, string? ConnectionString); public record DeleteDatabaseConnectionDefCommand(int DatabaseConnectionId); diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 239c6ed2..ae49d023 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -2637,16 +2637,30 @@ public class ManagementActor : ReceiveActor // Database Connection Definition handlers // ======================================================================== + /// + /// Secret-elided DatabaseConnectionDefinition projection (arch-review C3): the + /// ADO.NET ConnectionString can embed the SQL password and must never + /// leave this boundary via List/Get responses. Presence is surfaced as + /// HasConnectionString. Mirrors . + /// + private static object DatabaseConnectionPublicShape(DatabaseConnectionDefinition d) => new + { + d.Id, + d.Name, + HasConnectionString = !string.IsNullOrEmpty(d.ConnectionString), + }; + private static async Task HandleListDatabaseConnections(IServiceProvider sp) { var repo = sp.GetRequiredService(); - return await repo.GetAllDatabaseConnectionsAsync(); + return (await repo.GetAllDatabaseConnectionsAsync()).Select(DatabaseConnectionPublicShape).ToList(); } private static async Task HandleGetDatabaseConnection(IServiceProvider sp, GetDatabaseConnectionCommand cmd) { var repo = sp.GetRequiredService(); - return await repo.GetDatabaseConnectionByIdAsync(cmd.DatabaseConnectionId); + var def = await repo.GetDatabaseConnectionByIdAsync(cmd.DatabaseConnectionId); + return def is null ? null : DatabaseConnectionPublicShape(def); } private static async Task HandleCreateDatabaseConnection(IServiceProvider sp, CreateDatabaseConnectionDefCommand cmd, string user) @@ -2656,7 +2670,7 @@ public class ManagementActor : ReceiveActor await repo.AddDatabaseConnectionAsync(def); await repo.SaveChangesAsync(); await AuditAsync(sp, user, "Create", "DatabaseConnection", def.Id.ToString(), def.Name, new { def.Id, def.Name }); - return def; + return DatabaseConnectionPublicShape(def); } private static async Task HandleUpdateDatabaseConnection(IServiceProvider sp, UpdateDatabaseConnectionDefCommand cmd, string user) @@ -2665,11 +2679,14 @@ public class ManagementActor : ReceiveActor var def = await repo.GetDatabaseConnectionByIdAsync(cmd.DatabaseConnectionId) ?? throw new ManagementCommandException($"DatabaseConnection with ID {cmd.DatabaseConnectionId} not found."); def.Name = cmd.Name; - def.ConnectionString = cmd.ConnectionString; + // Preserve-if-null: an update that omits ConnectionString leaves the stored + // secret intact — the connection string is elided from List/Get, so a + // round-trip must not be required to re-send it. + if (cmd.ConnectionString is not null) def.ConnectionString = cmd.ConnectionString; await repo.UpdateDatabaseConnectionAsync(def); await repo.SaveChangesAsync(); await AuditAsync(sp, user, "Update", "DatabaseConnection", def.Id.ToString(), def.Name, new { def.Id, def.Name }); - return def; + return DatabaseConnectionPublicShape(def); } private static async Task HandleDeleteDatabaseConnection(IServiceProvider sp, DeleteDatabaseConnectionDefCommand cmd, string user) diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecretProjectionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecretProjectionTests.cs index 1b276fae..4b3d0289 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecretProjectionTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecretProjectionTests.cs @@ -218,4 +218,59 @@ public class SecretProjectionTests : TestKit, IDisposable Arg.Is(o => !ManagementActor.SerializeResult(o).Contains("newsecret")), Arg.Any()); } + + // ======================================================================== + // Task 9 — DatabaseConnection ConnectionString elision + // ======================================================================== + + private IExternalSystemRepository AddDatabaseConnectionRepo(DatabaseConnectionDefinition def) + { + var repo = Substitute.For(); + repo.GetDatabaseConnectionByIdAsync(def.Id, Arg.Any()).Returns(def); + repo.GetAllDatabaseConnectionsAsync(Arg.Any()) + .Returns(new List { def }); + _services.AddScoped(_ => repo); + return repo; + } + + [Fact] + public void GetDatabaseConnection_ResponseOmitsConnectionString() + { + AddDatabaseConnectionRepo(new DatabaseConnectionDefinition("db", "Server=db;Password=sql-secret") { Id = 1 }); + + var actor = CreateActor(); + actor.Tell(Envelope(new GetDatabaseConnectionCommand(1), "Viewer")); + + var resp = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.DoesNotContain("sql-secret", resp.JsonData); + Assert.Contains("hasConnectionString", resp.JsonData); + } + + [Fact] + public void ListDatabaseConnections_ResponseOmitsConnectionString() + { + AddDatabaseConnectionRepo(new DatabaseConnectionDefinition("db", "Server=db;Password=sql-secret") { Id = 1 }); + + var actor = CreateActor(); + actor.Tell(Envelope(new ListDatabaseConnectionsCommand(), "Viewer")); + + var resp = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.DoesNotContain("sql-secret", resp.JsonData); + Assert.Contains("hasConnectionString", resp.JsonData); + } + + [Fact] + public async Task UpdateDatabaseConnection_NullConnectionString_PreservesStoredSecret() + { + var stored = new DatabaseConnectionDefinition("db", "Server=db;Password=sql-secret") { Id = 1 }; + var repo = AddDatabaseConnectionRepo(stored); + + var actor = CreateActor(); + actor.Tell(Envelope(new UpdateDatabaseConnectionDefCommand(1, "db-renamed", null), "Designer")); + ExpectMsg(TimeSpan.FromSeconds(5)); + + await repo.Received(1).UpdateDatabaseConnectionAsync( + Arg.Is(d => d.ConnectionString == "Server=db;Password=sql-secret"), + Arg.Any()); + } }