diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index 7fa5cd1fec..b78cea533e 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -733,7 +733,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}")) } // --------------------------------------------------------------------------- @@ -948,3 +949,18 @@ pub fn scrub_git_env(command: &mut Command) -> &mut Command { } command } + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + #[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")); + } +} diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index a2ad725a4c..fb7349c6cf 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -7699,7 +7699,7 @@ pub async fn sandbox_logs( .as_millis(), ) .into_diagnostic()?; - now_ms - dur_ms + now_ms.saturating_sub(dur_ms) } else { 0 }; diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 2f89c2229e..ce01d20dfe 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2689,7 +2689,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)] @@ -2735,7 +2743,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( diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 67036b2192..7821901ffe 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -2226,3 +2226,45 @@ 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")); +} + +#[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 4ca2575d82..d36964fc94 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -987,7 +987,16 @@ pub fn build_container_spec_with_token_and_gpu_devices( openshell_core::config::DEFAULT_SSH_PORT ), ], - interval: config.health_check_interval_secs * 1_000_000_000, + 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!( + "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, @@ -1107,8 +1116,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. @@ -1183,6 +1197,41 @@ 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_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)); diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 57bf421b88..b5a604228d 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -1421,7 +1421,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 }; diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index b3937e2b90..cd86931781 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), "-"); + } +}