From e3b83f856121aaabb93302f32fa7dcfc4b5d9067 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 19 Jun 2026 03:24:11 -0400 Subject: [PATCH] fix(cli): guard min-time overflow + normalize 0 exec-timeout to null + stale comment (#54 review) --- .../Commands/TemplateCommands.cs | 8 +++++ .../Pages/Design/TemplateEdit.razor | 3 +- .../ManagementActor.cs | 4 +-- .../Commands/TemplateScriptTimingTests.cs | 30 +++++++++++++++++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs index 90404451..b9a1d1c3 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs @@ -395,6 +395,14 @@ public static class TemplateCommands return false; } + // Guard against overflow: reject if amount * factorMs would exceed TimeSpan.MaxValue (in ms). + // The divide is safe because factorMs is always >= 1. + if (amount > TimeSpan.MaxValue.TotalMilliseconds / factorMs) + { + error = $"Invalid --min-time-between-runs '{value}': value is too large."; + return false; + } + // 0 (with or without a unit) → unset, mirroring DurationInput.Compose's non-positive handling. duration = amount == 0 ? null : TimeSpan.FromMilliseconds(amount * factorMs); return true; diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TemplateEdit.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TemplateEdit.razor index dce0f573..a618375a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TemplateEdit.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Design/TemplateEdit.razor @@ -2067,8 +2067,7 @@ _scriptParameters = script.ParameterDefinitions; _scriptReturn = script.ReturnDefinition; _scriptIsLocked = script.IsLocked; - // Preserve any timeout set via Transport import — the UI has no authoring - // control for this field, so we round-trip the loaded value unchanged. + // Load the per-script execution timeout into the authoring input (null = use site default). _scriptExecutionTimeoutSeconds = script.ExecutionTimeoutSeconds; _scriptModalTab = "trigger"; ResetScriptTestRun(); diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 821c16a6..afcc58f0 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -2172,7 +2172,7 @@ public class ManagementActor : ReceiveActor ParameterDefinitions = cmd.ParameterDefinitions, ReturnDefinition = cmd.ReturnDefinition, MinTimeBetweenRuns = cmd.MinTimeBetweenRuns, - ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds + ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds is > 0 ? cmd.ExecutionTimeoutSeconds : null }; var result = await svc.AddScriptAsync(cmd.TemplateId, script, user); return result.IsSuccess ? result.Value : throw new ManagementCommandException(result.Error); @@ -2189,7 +2189,7 @@ public class ManagementActor : ReceiveActor ParameterDefinitions = cmd.ParameterDefinitions, ReturnDefinition = cmd.ReturnDefinition, MinTimeBetweenRuns = cmd.MinTimeBetweenRuns, - ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds + ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds is > 0 ? cmd.ExecutionTimeoutSeconds : null }; var result = await svc.UpdateScriptAsync(cmd.ScriptId, script, user); return result.IsSuccess ? result.Value : throw new ManagementCommandException(result.Error); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/TemplateScriptTimingTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/TemplateScriptTimingTests.cs index 7d4f7a0f..8777c669 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/TemplateScriptTimingTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/TemplateScriptTimingTests.cs @@ -117,4 +117,34 @@ public class TemplateScriptTimingTests Assert.Null(d); Assert.NotNull(err); } + + /// + /// An absurdly large value (16-digit millisecond count) would overflow long/TimeSpan + /// without the overflow guard. The parser must return false cleanly — not throw. + /// + [Theory] + [InlineData("9999999999999999ms")] // 16-digit ms: far exceeds TimeSpan.MaxValue (~922337203685477ms) + [InlineData("9999999999999999s")] // 16-digit seconds: even larger when scaled + [InlineData("9999999999999999min")] // 16-digit minutes + public void MinTime_AbsurdlyLarge_ReturnsFalseWithoutThrowing(string value) + { + // Must not throw — TryParse contract: never throws, always returns bool. + var threw = false; + bool result = false; + TimeSpan? duration = null; + string? error = null; + try + { + result = TemplateCommands.TryParseMinTimeBetweenRuns(value, out duration, out error); + } + catch + { + threw = true; + } + + Assert.False(threw, "TryParseMinTimeBetweenRuns must not throw on large input"); + Assert.False(result); + Assert.Null(duration); + Assert.NotNull(error); + } }