fix(cli): guard min-time overflow + normalize 0 exec-timeout to null + stale comment (#54 review)

This commit is contained in:
Joseph Doherty
2026-06-19 03:24:11 -04:00
parent 597d664a53
commit e3b83f8561
4 changed files with 41 additions and 4 deletions
@@ -395,6 +395,14 @@ public static class TemplateCommands
return false; 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. // 0 (with or without a unit) → unset, mirroring DurationInput.Compose's non-positive handling.
duration = amount == 0 ? null : TimeSpan.FromMilliseconds(amount * factorMs); duration = amount == 0 ? null : TimeSpan.FromMilliseconds(amount * factorMs);
return true; return true;
@@ -2067,8 +2067,7 @@
_scriptParameters = script.ParameterDefinitions; _scriptParameters = script.ParameterDefinitions;
_scriptReturn = script.ReturnDefinition; _scriptReturn = script.ReturnDefinition;
_scriptIsLocked = script.IsLocked; _scriptIsLocked = script.IsLocked;
// Preserve any timeout set via Transport import — the UI has no authoring // Load the per-script execution timeout into the authoring input (null = use site default).
// control for this field, so we round-trip the loaded value unchanged.
_scriptExecutionTimeoutSeconds = script.ExecutionTimeoutSeconds; _scriptExecutionTimeoutSeconds = script.ExecutionTimeoutSeconds;
_scriptModalTab = "trigger"; _scriptModalTab = "trigger";
ResetScriptTestRun(); ResetScriptTestRun();
@@ -2172,7 +2172,7 @@ public class ManagementActor : ReceiveActor
ParameterDefinitions = cmd.ParameterDefinitions, ParameterDefinitions = cmd.ParameterDefinitions,
ReturnDefinition = cmd.ReturnDefinition, ReturnDefinition = cmd.ReturnDefinition,
MinTimeBetweenRuns = cmd.MinTimeBetweenRuns, MinTimeBetweenRuns = cmd.MinTimeBetweenRuns,
ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds is > 0 ? cmd.ExecutionTimeoutSeconds : null
}; };
var result = await svc.AddScriptAsync(cmd.TemplateId, script, user); var result = await svc.AddScriptAsync(cmd.TemplateId, script, user);
return result.IsSuccess ? result.Value : throw new ManagementCommandException(result.Error); return result.IsSuccess ? result.Value : throw new ManagementCommandException(result.Error);
@@ -2189,7 +2189,7 @@ public class ManagementActor : ReceiveActor
ParameterDefinitions = cmd.ParameterDefinitions, ParameterDefinitions = cmd.ParameterDefinitions,
ReturnDefinition = cmd.ReturnDefinition, ReturnDefinition = cmd.ReturnDefinition,
MinTimeBetweenRuns = cmd.MinTimeBetweenRuns, MinTimeBetweenRuns = cmd.MinTimeBetweenRuns,
ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds ExecutionTimeoutSeconds = cmd.ExecutionTimeoutSeconds is > 0 ? cmd.ExecutionTimeoutSeconds : null
}; };
var result = await svc.UpdateScriptAsync(cmd.ScriptId, script, user); var result = await svc.UpdateScriptAsync(cmd.ScriptId, script, user);
return result.IsSuccess ? result.Value : throw new ManagementCommandException(result.Error); return result.IsSuccess ? result.Value : throw new ManagementCommandException(result.Error);
@@ -117,4 +117,34 @@ public class TemplateScriptTimingTests
Assert.Null(d); Assert.Null(d);
Assert.NotNull(err); Assert.NotNull(err);
} }
/// <summary>
/// An absurdly large value (16-digit millisecond count) would overflow long/TimeSpan
/// without the overflow guard. The parser must return false cleanly — not throw.
/// </summary>
[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);
}
} }