From 5f7987b480606405ce582531719d1f4495fdf037 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 05:28:49 -0500 Subject: [PATCH 1/9] fix(server): avoid i64 overflow in SSH session TTL The SSH session expiry calculation cast a u64 TTL to i64 and multiplied by 1000 without bounds checking. A configured TTL larger than i64::MAX / 1000 overflows and panics in debug builds. Use i64::try_from with saturating_mul and saturating_add so any out-of-range TTL clamps to i64::MAX milliseconds. Signed-off-by: Andrew White --- crates/openshell-server/src/grpc/sandbox.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 0d6a4e277..903070baa 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -1422,7 +1422,10 @@ pub(super) async fn handle_create_ssh_session( let token = uuid::Uuid::new_v4().to_string(); let now_ms = current_time_ms(); let expires_at_ms = if state.config.ssh_session_ttl_secs > 0 { - now_ms + (state.config.ssh_session_ttl_secs as i64 * 1000) + let ttl_ms = i64::try_from(state.config.ssh_session_ttl_secs) + .unwrap_or(i64::MAX) + .saturating_mul(1000); + now_ms.saturating_add(ttl_ms) } else { 0 }; From 8fa1df1113edc712df2522a8fce8fb437c232570 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 05:28:49 -0500 Subject: [PATCH 2/9] fix(driver-podman): avoid u64 overflow in health-check interval The Podman health-check interval multiplied a u64 seconds value by 1_000_000_000 to produce nanoseconds. A configured interval above u64::MAX / 1e9 overflows. Use saturating_mul so huge values clamp to u64::MAX nanoseconds instead of panicking. Signed-off-by: Andrew White --- crates/openshell-driver-podman/src/container.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 90ef0fec2..ab6a7c566 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -1074,7 +1074,9 @@ pub fn build_container_spec_for_image( openshell_core::config::DEFAULT_SSH_PORT ), ], - interval: config.health_check_interval_secs * 1_000_000_000, + interval: config + .health_check_interval_secs + .saturating_mul(1_000_000_000), timeout: 2_000_000_000, retries: 10, start_period: 5_000_000_000, From 4f80bc9ac3a1981b5410982f67a291f2f5fc624f Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 05:28:49 -0500 Subject: [PATCH 3/9] fix(cli): reject overflow in --since duration values parse_duration_to_ms parsed the numeric part as i64 and multiplied it by a fixed multiplier, which can overflow for large user inputs such as '100000000000000h'. Use checked_mul and return a clear error instead of panicking. Add a regression test. Signed-off-by: Andrew White --- crates/openshell-cli/src/commands/common.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index e6edb4d33..b8b467118 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -736,7 +736,8 @@ pub fn parse_duration_to_ms(s: &str) -> Result { )); } }; - Ok(num * multiplier) + num.checked_mul(multiplier) + .ok_or_else(|| miette::miette!("duration value is too large: {s}")) } // --------------------------------------------------------------------------- @@ -975,4 +976,10 @@ mod tests { let err = parse_duration_to_ms("\u{20ac}").expect_err("missing number should error"); assert!(err.to_string().contains("invalid duration")); } + + #[test] + fn parse_duration_to_ms_rejects_overflow() { + let err = parse_duration_to_ms("100000000000000h").expect_err("overflow should error"); + assert!(err.to_string().contains("too large")); + } } From 579613f0bfee254a135b8be0a466042cd18a545c Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 05:28:49 -0500 Subject: [PATCH 4/9] fix(cli): avoid underflow computing --since log start timestamp After parsing a duration, the CLI subtracted it from the current Unix epoch milliseconds. A duration larger than now_ms underflows. Use saturating_sub so the start timestamp clamps to 0 instead of panicking in debug builds. Signed-off-by: Andrew White --- crates/openshell-cli/src/run.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 4843475e5..2a322fdd1 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -6508,7 +6508,7 @@ pub async fn sandbox_logs( .as_millis(), ) .into_diagnostic()?; - now_ms - dur_ms + now_ms.saturating_sub(dur_ms) } else { 0 }; From 1d0308df69a606a9026c6e226957279b62add5ef Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 05:28:49 -0500 Subject: [PATCH 5/9] fix(tui): avoid underflow formatting future ages format_age subtracted created_secs from now before checking whether the timestamp was in the future, so a future or skewed timestamp underflowed before the guard could return '-'. Check created_secs > now before subtracting, and add regression tests for future, zero, and negative timestamps. Signed-off-by: Andrew White --- crates/openshell-tui/src/lib.rs | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index b3937e2b9..cd8693178 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -2603,11 +2603,10 @@ fn format_age(epoch_ms: i64) -> String { let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map_or(0, |d| d.as_secs().cast_signed()); - let diff = now - created_secs; - if diff < 0 { + if created_secs > now { return String::from("-"); } - let diff = diff.cast_unsigned(); + let diff = (now - created_secs).cast_unsigned(); if diff < 60 { format!("{diff}s") } else if diff < 3600 { @@ -2649,3 +2648,31 @@ fn days_to_ymd(days: i64) -> (i64, i64, i64) { let y = if m <= 2 { y + 1 } else { y }; (y, m, d) } + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_age_handles_future_timestamp() { + let future_ms = i64::try_from( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis(), + ) + .unwrap() + + 10_000; + assert_eq!(format_age(future_ms), "-"); + } + + #[test] + fn format_age_handles_zero_and_negative() { + assert_eq!(format_age(0), "-"); + assert_eq!(format_age(-1), "-"); + } +} From 1e426fa58f3801d98a3b806c5a8707e471de3eca Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 05:42:11 -0500 Subject: [PATCH 6/9] fix(driver-podman): avoid f64 overflow in CPU limit parse_cpu_to_microseconds checked that the input cores value was finite, but the product cores * 100_000 could still overflow to infinity (e.g. '1e300'). An infinite product cast to u64 saturates to u64::MAX, producing a bogus quota. Reject the value when the product is non-finite or exceeds u64::MAX. Add a regression test. Signed-off-by: Andrew White --- crates/openshell-driver-podman/src/container.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index ab6a7c566..d7bb679c5 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -1213,8 +1213,13 @@ fn parse_cpu_to_microseconds(quantity: &str) -> Option { if cores <= 0.0 || cores.is_nan() || cores.is_infinite() { return None; } + let micros_f = cores * 100_000.0; + #[allow(clippy::cast_precision_loss)] + if !micros_f.is_finite() || micros_f > u64::MAX as f64 { + return None; + } #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] - let val = (cores * 100_000.0) as u64; + let val = micros_f as u64; val }; // A quota of 0 microseconds is invalid — treat as no limit. @@ -1289,6 +1294,12 @@ mod tests { assert_eq!(parse_cpu_to_microseconds("0.5"), Some(50_000)); } + #[test] + fn parse_cpu_huge_value_returns_none_instead_of_overflow() { + // A finite f64 whose product with 100_000 overflows to infinity. + assert_eq!(parse_cpu_to_microseconds("1e300"), None); + } + #[test] fn parse_memory_binary_suffixes() { assert_eq!(parse_memory_to_bytes("256Mi"), Some(256 * 1024 * 1024)); From cde9c5bea7264e14468c7a0cf08af2725b25a22e Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 05:42:11 -0500 Subject: [PATCH 7/9] fix(driver-docker): avoid f64 overflow in CPU and memory limits parse_cpu_limit and parse_memory_limit parsed user-supplied f64 values and multiplied them by large constants before casting to i64. A huge input could make the product infinite, silently saturating to i64::MAX. Check that the rounded product is finite and within i64 range before casting, and return a clear failed-precondition error. Add regression tests. Signed-off-by: Andrew White --- crates/openshell-driver-docker/src/lib.rs | 20 ++++++++++++++++++-- crates/openshell-driver-docker/src/tests.rs | 14 ++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index a196ba6ad..095279f5a 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2752,7 +2752,15 @@ fn parse_cpu_limit(value: &str) -> Result, Status> { )); } - Ok(Some((cores * 1_000_000_000.0).round() as i64)) + let nano_cpus = (cores * 1_000_000_000.0).round(); + #[allow(clippy::cast_precision_loss)] + if !nano_cpus.is_finite() || nano_cpus < i64::MIN as f64 || nano_cpus > i64::MAX as f64 { + return Err(Status::failed_precondition(format!( + "docker cpu_limit '{value}' is too large", + ))); + } + + Ok(Some(nano_cpus as i64)) } #[allow(clippy::cast_possible_truncation)] @@ -2798,7 +2806,15 @@ fn parse_memory_limit(value: &str) -> Result, Status> { } }; - Ok(Some((amount * multiplier).round() as i64)) + let bytes = (amount * multiplier).round(); + #[allow(clippy::cast_precision_loss)] + if !bytes.is_finite() || bytes < i64::MIN as f64 || bytes > i64::MAX as f64 { + return Err(Status::failed_precondition(format!( + "docker memory_limit '{value}' is too large", + ))); + } + + Ok(Some(bytes as i64)) } fn sandbox_from_container_summary(summary: &ContainerSummary) -> Option { diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index a86a9936f..06a44f7d0 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -2250,3 +2250,17 @@ fn container_state_needs_resume_matches_startable_states() { ); } } + +#[test] +fn parse_cpu_limit_rejects_overflow() { + let err = parse_cpu_limit("1e300").unwrap_err(); + assert!(err.message().contains("too large")); +} + +#[test] +fn parse_memory_limit_rejects_overflow() { + // 308 nines is finite as f64 (~1e308), but multiplying by Gi overflows to inf. + let huge = "9".repeat(308) + "Gi"; + let err = parse_memory_limit(&huge).unwrap_err(); + assert!(err.message().contains("too large")); +} From f94ce9f077844eef215e02dce05c4947846cff14 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 28 Jul 2026 14:53:13 -0500 Subject: [PATCH 8/9] fix: reject numeric boundaries that round into saturation Addresses gator-agent review feedback on #2462: - Docker parse_cpu_limit / parse_memory_limit: use >= i64::MAX as f64 so inputs rounding to 2^63 are rejected instead of saturating. - Podman parse_cpu_to_microseconds: use >= u64::MAX as f64 so inputs rounding to 2^64 are rejected instead of saturating. - Podman health-check interval: replace saturating_mul with checked_mul and reject values exceeding i64::MAX nanoseconds. - Add regression tests for the i64::MAX, u64::MAX, and health-check interval boundaries. Signed-off-by: Andrew White --- crates/openshell-driver-docker/src/lib.rs | 4 +- crates/openshell-driver-docker/src/tests.rs | 28 +++++++++++++ .../openshell-driver-podman/src/container.rs | 40 ++++++++++++++++++- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 095279f5a..85fcfa7cb 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2754,7 +2754,7 @@ fn parse_cpu_limit(value: &str) -> Result, Status> { let nano_cpus = (cores * 1_000_000_000.0).round(); #[allow(clippy::cast_precision_loss)] - if !nano_cpus.is_finite() || nano_cpus < i64::MIN as f64 || nano_cpus > i64::MAX as f64 { + if !nano_cpus.is_finite() || nano_cpus < i64::MIN as f64 || nano_cpus >= i64::MAX as f64 { return Err(Status::failed_precondition(format!( "docker cpu_limit '{value}' is too large", ))); @@ -2808,7 +2808,7 @@ fn parse_memory_limit(value: &str) -> Result, Status> { let bytes = (amount * multiplier).round(); #[allow(clippy::cast_precision_loss)] - if !bytes.is_finite() || bytes < i64::MIN as f64 || bytes > i64::MAX as f64 { + if !bytes.is_finite() || bytes < i64::MIN as f64 || bytes >= i64::MAX as f64 { return Err(Status::failed_precondition(format!( "docker memory_limit '{value}' is too large", ))); diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 06a44f7d0..321be2e5e 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -2264,3 +2264,31 @@ fn parse_memory_limit_rejects_overflow() { let err = parse_memory_limit(&huge).unwrap_err(); assert!(err.message().contains("too large")); } + +#[test] +fn parse_cpu_limit_rejects_i64_max_boundary() { + // 9_223_372_037 cores * 1e9 rounds to 2^63, which used to pass the > check + // and silently saturate to i64::MAX. It must now be rejected. + let err = parse_cpu_limit("9223372037").unwrap_err(); + assert!(err.message().contains("too large")); + + // One core below the boundary is still valid. + assert_eq!( + parse_cpu_limit("9223372036").unwrap(), + Some(9_223_372_036_000_000_000) + ); +} + +#[test] +fn parse_memory_limit_rejects_i64_max_boundary() { + // 8192 PiB = 2^63 bytes, which used to pass the > check and silently + // saturate to i64::MAX. It must now be rejected. + let err = parse_memory_limit("8192Pi").unwrap_err(); + assert!(err.message().contains("too large")); + + // One PiB below the boundary is still valid. + assert_eq!( + parse_memory_limit("8191Pi").unwrap(), + Some(8191 * 1024_i64.pow(5)) + ); +} diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index d7bb679c5..b3ba18f68 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -1076,7 +1076,14 @@ pub fn build_container_spec_for_image( ], interval: config .health_check_interval_secs - .saturating_mul(1_000_000_000), + .checked_mul(1_000_000_000) + .filter(|ns| *ns <= i64::MAX as u64) + .ok_or_else(|| { + ComputeDriverError::InvalidArgument(format!( + "health_check_interval_secs {} exceeds maximum allowed nanoseconds", + config.health_check_interval_secs + )) + })?, timeout: 2_000_000_000, retries: 10, start_period: 5_000_000_000, @@ -1215,7 +1222,7 @@ fn parse_cpu_to_microseconds(quantity: &str) -> Option { } let micros_f = cores * 100_000.0; #[allow(clippy::cast_precision_loss)] - if !micros_f.is_finite() || micros_f > u64::MAX as f64 { + if !micros_f.is_finite() || micros_f >= u64::MAX as f64 { return None; } #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] @@ -1300,6 +1307,35 @@ mod tests { assert_eq!(parse_cpu_to_microseconds("1e300"), None); } + #[test] + fn parse_cpu_rejects_u64_max_boundary() { + // 184_467_440_737_095_520 cores * 100_000 rounds to 2^64, which used + // to pass the > check and silently saturate to u64::MAX. It must now + // be rejected. + assert_eq!(parse_cpu_to_microseconds("184467440737095520"), None); + + // One core below the boundary is still valid. + assert_eq!( + parse_cpu_to_microseconds("184467440737095"), + Some(18_446_744_073_709_500_416) + ); + } + + #[test] + fn container_spec_rejects_health_check_interval_overflow() { + let sandbox = test_sandbox("test-id", "test-name"); + let mut config = test_config(); + // i64::MAX nanoseconds / 1_000_000_000 ns/s = 9_223_372_036 seconds. + // One second over must be rejected before it can saturate. + config.health_check_interval_secs = 9_223_372_037; + let err = try_build_container_spec_with_token(&sandbox, &config, None).unwrap_err(); + assert!( + matches!(err, ComputeDriverError::InvalidArgument(_)), + "expected InvalidArgument, got {err:?}" + ); + assert!(format!("{err}").contains("health_check_interval_secs")); + } + #[test] fn parse_memory_binary_suffixes() { assert_eq!(parse_memory_to_bytes("256Mi"), Some(256 * 1024 * 1024)); From 1789522df14313e5062aacd46c322590850dd088 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 29 Jul 2026 07:55:11 -0500 Subject: [PATCH 9/9] fix: address johntmyers review findings on overflow fixes - Docker CPU millicores: use checked multiplication and reject overflow. - Docker CPU/memory f64 boundaries: use exact equality tests (9223372036.854776 cores, 8192Pi memory) and add millicore boundary test. - Podman CPU: correct boundary test value to 184467440737095.51616 for exact 2^64 microseconds. - Podman health-check interval: pre-validate against i64::MAX / 1_000_000_000 seconds and use checked multiplication; add boundary acceptance test. - CLI duration parsing: reject negative durations; clamp overlong --since durations to zero instead of producing future timestamps. - Server SSH TTL: use checked conversion, multiplication, and addition, and return InvalidArgument instead of silently creating non-expiring sessions. --- crates/openshell-cli/src/commands/common.rs | 19 +++++++ crates/openshell-cli/src/run.rs | 5 +- crates/openshell-driver-docker/src/lib.rs | 9 +++- crates/openshell-driver-docker/src/tests.rs | 20 ++++++-- .../openshell-driver-podman/src/container.rs | 50 ++++++++++++++----- crates/openshell-server/src/grpc/sandbox.rs | 40 +++++++++++++-- 6 files changed, 121 insertions(+), 22 deletions(-) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index b8b467118..a134ff6f0 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -726,6 +726,11 @@ pub fn parse_duration_to_ms(s: &str) -> Result { let num: i64 = num_str .parse() .map_err(|_| miette::miette!("invalid duration: {s} (expected e.g. 5m, 1h, 30s)"))?; + if num < 0 { + return Err(miette::miette!( + "duration must not be negative: {s}" + )); + } let multiplier = match unit { "s" => 1_000, "m" => 60_000, @@ -982,4 +987,18 @@ mod tests { let err = parse_duration_to_ms("100000000000000h").expect_err("overflow should error"); assert!(err.to_string().contains("too large")); } + + #[test] + fn parse_duration_to_ms_rejects_negative() { + let err = parse_duration_to_ms("-5m").expect_err("negative duration should error"); + assert!(err.to_string().contains("negative")); + } + + #[test] + fn parse_duration_to_ms_accepts_zero_and_valid() { + assert_eq!(parse_duration_to_ms("0s").unwrap(), 0); + assert_eq!(parse_duration_to_ms("30s").unwrap(), 30_000); + assert_eq!(parse_duration_to_ms("5m").unwrap(), 300_000); + assert_eq!(parse_duration_to_ms("2h").unwrap(), 7_200_000); + } } diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 2a322fdd1..8622735bd 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -6508,7 +6508,10 @@ pub async fn sandbox_logs( .as_millis(), ) .into_diagnostic()?; - now_ms.saturating_sub(dur_ms) + // Negative durations are rejected by parse_duration_to_ms. Overlong + // durations are clamped to zero (show all logs since the epoch) rather + // than silently producing a future timestamp. + now_ms.checked_sub(dur_ms).unwrap_or(0).max(0) } else { 0 }; diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 85fcfa7cb..60b04ea2c 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2738,7 +2738,14 @@ fn parse_cpu_limit(value: &str) -> Result, Status> { "docker cpu_limit must be greater than zero", )); } - return Ok(Some(millicores.saturating_mul(1_000_000))); + return millicores + .checked_mul(1_000_000) + .map(Some) + .ok_or_else(|| { + Status::failed_precondition(format!( + "docker cpu_limit '{value}' is too large", + )) + }); } let cores = value.parse::().map_err(|_| { diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 321be2e5e..0e752d92d 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -2267,9 +2267,11 @@ fn parse_memory_limit_rejects_overflow() { #[test] fn parse_cpu_limit_rejects_i64_max_boundary() { - // 9_223_372_037 cores * 1e9 rounds to 2^63, which used to pass the > check - // and silently saturate to i64::MAX. It must now be rejected. - let err = parse_cpu_limit("9223372037").unwrap_err(); + // 9_223_372_036.854776 cores * 1e9 rounds exactly to 2^63, which used to + // pass the > check and silently saturate to i64::MAX. It must now be + // rejected. (9_223_372_037 is already above the boundary and would also + // pass a > guard, so it does not test the equality case.) + let err = parse_cpu_limit("9223372036.854776").unwrap_err(); assert!(err.message().contains("too large")); // One core below the boundary is still valid. @@ -2279,6 +2281,18 @@ fn parse_cpu_limit_rejects_i64_max_boundary() { ); } +#[test] +fn parse_cpu_limit_rejects_millicore_overflow() { + // 9_223_372_036_854 millicores * 1_000_000 == 9_223_372_036_854_000_000, + // which fits in i64. One millicore more overflows and must be rejected. + assert_eq!( + parse_cpu_limit("9223372036854m").unwrap(), + Some(9_223_372_036_854_000_000) + ); + let err = parse_cpu_limit("9223372036855m").unwrap_err(); + assert!(err.message().contains("too large")); +} + #[test] fn parse_memory_limit_rejects_i64_max_boundary() { // 8192 PiB = 2^63 bytes, which used to pass the > check and silently diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index b3ba18f68..eceaa3177 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -1074,16 +1074,25 @@ pub fn build_container_spec_for_image( openshell_core::config::DEFAULT_SSH_PORT ), ], - interval: config - .health_check_interval_secs - .checked_mul(1_000_000_000) - .filter(|ns| *ns <= i64::MAX as u64) - .ok_or_else(|| { - ComputeDriverError::InvalidArgument(format!( + interval: { + const NS_PER_S: u64 = 1_000_000_000; + let max_secs = (i64::MAX as u64) / NS_PER_S; + if config.health_check_interval_secs > max_secs { + return Err(ComputeDriverError::InvalidArgument(format!( "health_check_interval_secs {} exceeds maximum allowed nanoseconds", config.health_check_interval_secs - )) - })?, + ))); + } + config + .health_check_interval_secs + .checked_mul(NS_PER_S) + .ok_or_else(|| { + ComputeDriverError::InvalidArgument(format!( + "health_check_interval_secs {} exceeds maximum allowed nanoseconds", + config.health_check_interval_secs + )) + })? + }, timeout: 2_000_000_000, retries: 10, start_period: 5_000_000_000, @@ -1309,12 +1318,13 @@ mod tests { #[test] fn parse_cpu_rejects_u64_max_boundary() { - // 184_467_440_737_095_520 cores * 100_000 rounds to 2^64, which used - // to pass the > check and silently saturate to u64::MAX. It must now - // be rejected. - assert_eq!(parse_cpu_to_microseconds("184467440737095520"), None); + // 184_467_440_737_095.51616 cores * 100_000 rounds exactly to 2^64, + // which used to pass the > check and silently saturate to u64::MAX. + // It must now be rejected. The previous value (184467440737095520) + // was ~1000x larger and did not exercise the equality boundary. + assert_eq!(parse_cpu_to_microseconds("184467440737095.51616"), None); - // One core below the boundary is still valid. + // Just below the boundary is still valid. assert_eq!( parse_cpu_to_microseconds("184467440737095"), Some(18_446_744_073_709_500_416) @@ -1336,6 +1346,20 @@ mod tests { assert!(format!("{err}").contains("health_check_interval_secs")); } + #[test] + fn container_spec_accepts_health_check_interval_at_boundary() { + let sandbox = test_sandbox("test-id", "test-name"); + let mut config = test_config(); + // i64::MAX nanoseconds / 1_000_000_000 ns/s = 9_223_372_036 seconds. + const NS_PER_S: u64 = 1_000_000_000; + config.health_check_interval_secs = (i64::MAX as u64) / NS_PER_S; + let spec = try_build_container_spec_with_token(&sandbox, &config, None).unwrap(); + let interval = spec["healthconfig"]["Interval"] + .as_u64() + .expect("healthcheck interval should be a u64"); + assert_eq!(interval, config.health_check_interval_secs * NS_PER_S); + } + #[test] fn parse_memory_binary_suffixes() { assert_eq!(parse_memory_to_bytes("256Mi"), Some(256 * 1024 * 1024)); diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 903070baa..06a0362d6 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -1422,10 +1422,14 @@ pub(super) async fn handle_create_ssh_session( let token = uuid::Uuid::new_v4().to_string(); let now_ms = current_time_ms(); let expires_at_ms = if state.config.ssh_session_ttl_secs > 0 { - let ttl_ms = i64::try_from(state.config.ssh_session_ttl_secs) - .unwrap_or(i64::MAX) - .saturating_mul(1000); - now_ms.saturating_add(ttl_ms) + let ttl_secs = state.config.ssh_session_ttl_secs; + let ttl_ms = i64::try_from(ttl_secs) + .map_err(|_| Status::invalid_argument("ssh_session_ttl_secs is too large"))? + .checked_mul(1000) + .ok_or_else(|| Status::invalid_argument("ssh_session_ttl_secs is too large"))?; + now_ms + .checked_add(ttl_ms) + .ok_or_else(|| Status::invalid_argument("ssh_session_ttl_secs is too large"))? } else { 0 }; @@ -4019,4 +4023,32 @@ mod tests { assert!(session.revoked); assert_eq!(session.object_workspace(), "default"); } + + #[tokio::test] + async fn create_ssh_session_rejects_too_large_ttl() { + let mut state = test_server_state().await; + state + .store + .put_message(&test_sandbox("ttl-test", Vec::new())) + .await + .unwrap(); + + // Any value larger than i64::MAX seconds cannot be converted to + // milliseconds without overflowing. + Arc::get_mut(&mut state) + .expect("fresh test state is uniquely held") + .config + .ssh_session_ttl_secs = u64::MAX; + + let err = handle_create_ssh_session( + &state, + Request::new(CreateSshSessionRequest { + sandbox_id: "sandbox-ttl-test".to_string(), + }), + ) + .await + .unwrap_err(); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + assert!(err.message().contains("ssh_session_ttl_secs is too large")); + } }