diff --git a/docs/requirements/Component-CLI.md b/docs/requirements/Component-CLI.md index 9ea5d34f..77eef4f3 100644 --- a/docs/requirements/Component-CLI.md +++ b/docs/requirements/Component-CLI.md @@ -162,6 +162,8 @@ scadabridge deploy artifacts [--site-id ] scadabridge deploy status [--instance-id ] [--status ] [--page ] [--page-size ] ``` +`deploy artifacts` (and its alias `site deploy-artifacts`): `--site-id` is now honored — supplying it deploys system-wide artifacts to exactly that site; omitting it deploys fleet-wide to every site. A site-scoped (non-Administrator) Deployer must supply an in-scope `--site-id`; fleet-wide deployment (no `--site-id`) requires a system-wide Deployer or Administrator (arch-review C2). + ### Data Connection Commands ``` scadabridge data-connection list [--site-id ] diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 188b9596..f3667e0a 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -139,7 +139,7 @@ Both endpoints honour any site-scope rules attached to the caller's audit role b ### Deployments - **DeployInstance**: Deploy configuration to a specific instance (includes pre-deployment validation). -- **DeployArtifacts**: Deploy system-wide artifacts (shared scripts, external system definitions, DB connections, data connections) to all sites or a specific site. +- **DeployArtifacts**: Deploy system-wide artifacts (shared scripts, external system definitions, DB connections, data connections) to all sites or a specific site. The command's `SiteId` is honored (arch-review C2): a value routes to the single-site deploy path (`ArtifactDeploymentService.DeployToSiteAsync`), while `null` deploys fleet-wide (`DeployToAllSitesAsync`). Site scope is enforced — a site-scoped (non-Administrator) Deployer may only target a site within its `PermittedSiteIds`, and may **not** deploy fleet-wide (`SiteId is null` is rejected with `SiteScopeViolationException` → `ManagementUnauthorized`, since `EnforceSiteScope(null)` is a deliberate no-op); fleet-wide deployment requires a system-wide Deployer or Administrator. - **GetDeploymentStatus**: Query deployment status. ### Secured Writes (MxGateway, two-person) diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index 6f5dd2f8..8322bf03 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -760,7 +760,7 @@ scadabridge --url site deploy-artifacts [--site-id ] | Option | Required | Description | |--------|----------|-------------| -| `--site-id` | no | Target site ID; omit to deploy to all sites | +| `--site-id` | no | Target site ID; deploys system-wide artifacts to exactly that site. Omit to deploy fleet-wide to all sites. A site-scoped (non-Administrator) Deployer must supply an in-scope `--site-id`; fleet-wide deployment requires a system-wide Deployer or Administrator (arch-review C2). | #### `site area list` @@ -840,7 +840,7 @@ scadabridge --url deploy artifacts [--site-id ] | Option | Required | Description | |--------|----------|-------------| -| `--site-id` | no | Target site ID; omit for all sites | +| `--site-id` | no | Target site ID; deploys to exactly that site. Omit to deploy fleet-wide. A site-scoped (non-Administrator) Deployer must supply an in-scope `--site-id`; fleet-wide requires a system-wide Deployer or Administrator (arch-review C2). | #### `deploy status` diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs index cdcb99a3..33cfd3f1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs @@ -225,7 +225,7 @@ public class ArtifactDeploymentService /// The user initiating the deployment. /// Cancellation token. /// A summary of the artifact deployment results for all sites. - public async Task> DeployToAllSitesAsync( + public virtual async Task> DeployToAllSitesAsync( string user, CancellationToken cancellationToken = default) { @@ -243,7 +243,7 @@ public class ArtifactDeploymentService /// The user initiating the deployment. /// Cancellation token. /// A summary of the artifact deployment result for the single site. - public async Task> DeployToSiteAsync( + public virtual async Task> DeployToSiteAsync( int siteId, string user, CancellationToken cancellationToken = default) diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 4969ef77..9aee5c93 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -433,7 +433,7 @@ public class ManagementActor : ReceiveActor DeleteScopeRuleCommand cmd => await HandleDeleteScopeRule(sp, cmd, user.Username), // Deployments - MgmtDeployArtifactsCommand cmd => await HandleDeployArtifacts(sp, cmd, user.Username), + MgmtDeployArtifactsCommand cmd => await HandleDeployArtifacts(sp, cmd, user), QueryDeploymentsCommand cmd => await HandleQueryDeployments(sp, cmd, user), GetDeploymentDiffCommand cmd => await HandleGetDeploymentDiff(sp, cmd, user), @@ -2205,10 +2205,24 @@ public class ManagementActor : ReceiveActor // Deployment handlers // ======================================================================== - private static async Task HandleDeployArtifacts(IServiceProvider sp, MgmtDeployArtifactsCommand cmd, string user) + private static async Task HandleDeployArtifacts(IServiceProvider sp, MgmtDeployArtifactsCommand cmd, AuthenticatedUser user) { + // A site-scoped Deployer may not trigger a FLEET-WIDE artifact deployment: + // EnforceSiteScope(null) is a no-op by design, so the fleet-wide case needs + // its own guard (arch-review C2 authorization gap). + if (cmd.SiteId is null + && user.PermittedSiteIds.Length > 0 + && !user.Roles.Contains(Roles.Administrator, StringComparer.OrdinalIgnoreCase)) + { + throw new SiteScopeViolationException( + "Site-scoped users cannot deploy artifacts fleet-wide; specify a target site id."); + } + EnforceSiteScope(user, cmd.SiteId); + var svc = sp.GetRequiredService(); - var result = await svc.DeployToAllSitesAsync(user); + var result = cmd.SiteId is int siteId + ? await svc.DeployToSiteAsync(siteId, user.Username) + : await svc.DeployToAllSitesAsync(user.Username); return result.IsSuccess ? result.Value : throw new ManagementCommandException(result.Error); diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs index 7c29dd2e..0ac90f32 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs @@ -29,6 +29,7 @@ public class ManagementActorTests : TestKit, IDisposable private readonly ITemplateEngineRepository _templateRepo; private readonly IAuditService _auditService; private readonly ServiceCollection _services; + private ArtifactDeploymentService _artifactService = null!; public ManagementActorTests() { @@ -56,6 +57,35 @@ public class ManagementActorTests : TestKit, IDisposable new(new AuthenticatedUser("scopeduser", "Scoped User", roles, permittedSiteIds), command, Guid.NewGuid().ToString("N")); + /// + /// Registers a substitutable into the + /// harness DI so DeployArtifacts tests can verify which deploy path was taken + /// (single-site vs. fleet-wide) without exercising the real cluster comms. + /// The virtual DeployToSiteAsync/DeployToAllSitesAsync members are + /// intercepted; all other members keep their real (unused) bodies. + /// + private void RegisterArtifactService() + { + var comms = new CommunicationService( + Options.Create(new CommunicationOptions()), + NullLogger.Instance); + _artifactService = Substitute.For( + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + comms, + Substitute.For(), + Options.Create(new DeploymentManagerOptions()), + NullLogger.Instance); + _services.AddScoped(_ => _artifactService); + } + + private static Result DeploySummary() => + Result.Success( + new ArtifactDeploymentSummary("deploy-1", Array.Empty(), 1, 0)); + void IDisposable.Dispose() { Shutdown(); @@ -1285,6 +1315,69 @@ public class ManagementActorTests : TestKit, IDisposable Assert.Contains("deploy-2", response.JsonData); } + // ======================================================================== + // DeployArtifacts honors SiteId + site-scope enforcement (arch-review C2) + // + // MgmtDeployArtifactsCommand.SiteId is now honored: a value routes to the + // single-site deploy path; null routes fleet-wide. A site-scoped (non-Admin) + // Deployer may not deploy fleet-wide, and may not target a site outside scope. + // ======================================================================== + + [Fact] + public void DeployArtifacts_WithSiteId_CallsSingleSiteDeploy() + { + RegisterArtifactService(); + _artifactService.DeployToSiteAsync(7, "testuser", Arg.Any()) + .Returns(DeploySummary()); + + var actor = CreateActor(); + actor.Tell(Envelope(new MgmtDeployArtifactsCommand(SiteId: 7), "Deployer")); + + ExpectMsg(TimeSpan.FromSeconds(5)); + _artifactService.Received(1).DeployToSiteAsync(7, "testuser", Arg.Any()); + _artifactService.DidNotReceiveWithAnyArgs().DeployToAllSitesAsync(default!, default); + } + + [Fact] + public void DeployArtifacts_FleetWide_CallsAllSitesDeploy() + { + RegisterArtifactService(); + _artifactService.DeployToAllSitesAsync("testuser", Arg.Any()) + .Returns(DeploySummary()); + + var actor = CreateActor(); + actor.Tell(Envelope(new MgmtDeployArtifactsCommand(SiteId: null), "Deployer")); + + ExpectMsg(TimeSpan.FromSeconds(5)); + _artifactService.Received(1).DeployToAllSitesAsync("testuser", Arg.Any()); + _artifactService.DidNotReceiveWithAnyArgs().DeployToSiteAsync(default, default!, default); + } + + [Fact] + public void DeployArtifacts_SiteScopedUser_OutOfScopeSite_Unauthorized() + { + RegisterArtifactService(); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new MgmtDeployArtifactsCommand(SiteId: 7), new[] { "3" }, "Deployer")); + + ExpectMsg(TimeSpan.FromSeconds(5)); + _artifactService.DidNotReceiveWithAnyArgs().DeployToSiteAsync(default, default!, default); + } + + [Fact] + public void DeployArtifacts_SiteScopedUser_FleetWide_Unauthorized() + { + RegisterArtifactService(); + + var actor = CreateActor(); + actor.Tell(ScopedEnvelope(new MgmtDeployArtifactsCommand(SiteId: null), new[] { "3" }, "Deployer")); + + var resp = ExpectMsg(TimeSpan.FromSeconds(5)); + Assert.Contains("fleet-wide", resp.Message, StringComparison.OrdinalIgnoreCase); + _artifactService.DidNotReceiveWithAnyArgs().DeployToAllSitesAsync(default!, default); + } + // ======================================================================== // SetInstanceOverrides atomicity (finding -015) //