From 4f1721a157438a33edde04ed62fd3eaba6986390 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Sun, 19 Jul 2026 22:48:55 -0700 Subject: [PATCH 1/3] refactor(cli): extract shared helpers into commands/common module Move shared formatting, parsing, display, and settings helpers from run.rs into a new commands/common.rs module. run.rs re-exports all public items so no callers change. This establishes the commands/ directory structure for the incremental run.rs split (#2304). Signed-off-by: Varsha Prasad Narsing Signed-off-by: Varsha Prasad Narsing --- crates/openshell-cli/src/commands/common.rs | 946 ++++++++++++++++++++ crates/openshell-cli/src/commands/mod.rs | 4 + crates/openshell-cli/src/lib.rs | 1 + crates/openshell-cli/src/run.rs | 940 +------------------ 4 files changed, 978 insertions(+), 913 deletions(-) create mode 100644 crates/openshell-cli/src/commands/common.rs create mode 100644 crates/openshell-cli/src/commands/mod.rs diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs new file mode 100644 index 0000000000..b8fce0bc68 --- /dev/null +++ b/crates/openshell-cli/src/commands/common.rs @@ -0,0 +1,946 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Shared helpers, types, and parsing utilities used across CLI command groups. + +use chrono::DateTime; +use dialoguer::{Confirm, theme::ColorfulTheme}; +use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use miette::{IntoDiagnostic, Result}; +use openshell_core::progress::{ + PROGRESS_ACTIVE_DETAIL_KEY, PROGRESS_ACTIVE_STEP_KEY, PROGRESS_COMPLETE_LABEL_KEY, + PROGRESS_COMPLETE_STEP_KEY, PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, + PROGRESS_STEP_STARTING_SANDBOX, +}; +use openshell_core::proto::{ + PlatformEvent, SandboxPhase, SandboxPolicy, SettingValue, setting_value, +}; +use openshell_core::settings::{self, SettingValueKind}; +use owo_colors::OwoColorize; +use std::collections::HashMap; +use std::io::IsTerminal; +use std::process::Command; +use std::time::{Duration, Instant}; + +// --------------------------------------------------------------------------- +// View types +// --------------------------------------------------------------------------- + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum PolicyGetView { + Metadata, + Base, + Full, +} + +impl PolicyGetView { + pub fn from_flags(base: bool, full: bool) -> Self { + match (base, full) { + (true, _) => Self::Base, + (false, true) => Self::Full, + (false, false) => Self::Metadata, + } + } + + pub fn includes_policy(self) -> bool { + matches!(self, Self::Base | Self::Full) + } +} + +// --------------------------------------------------------------------------- +// Formatting / display helpers +// --------------------------------------------------------------------------- + +/// Convert a sandbox phase integer to a human-readable string. +pub fn phase_name(phase: i32) -> &'static str { + match SandboxPhase::try_from(phase) { + Ok(SandboxPhase::Unspecified) => "Unspecified", + Ok(SandboxPhase::Provisioning) => "Provisioning", + Ok(SandboxPhase::Ready) => "Ready", + Ok(SandboxPhase::Error) => "Error", + Ok(SandboxPhase::Deleting) => "Deleting", + Ok(SandboxPhase::Unknown) | Err(_) => "Unknown", + } +} + +/// Format milliseconds since Unix epoch as a `YYYY-MM-DD HH:MM:SS` UTC string. +pub fn format_epoch_ms(ms: i64) -> String { + use std::time::UNIX_EPOCH; + + let Ok(ms_u64) = u64::try_from(ms) else { + return "-".to_string(); + }; + let Ok(time) = UNIX_EPOCH + .checked_add(Duration::from_millis(ms_u64)) + .ok_or(()) + else { + return "-".to_string(); + }; + let Ok(dur) = time.duration_since(UNIX_EPOCH) else { + return "-".to_string(); + }; + + let secs = dur.as_secs(); + let days = secs / 86400; + let time_of_day = secs % 86400; + let hours = time_of_day / 3600; + let minutes = (time_of_day % 3600) / 60; + let seconds = time_of_day % 60; + + let (y, m, d) = civil_from_days(days); + format!("{y:04}-{m:02}-{d:02} {hours:02}:{minutes:02}:{seconds:02}") +} + +/// Convert days since 1970-01-01 to (year, month, day). +/// Algorithm from Howard Hinnant's `chrono`-compatible date library. +fn civil_from_days(days: u64) -> (i64, u64, u64) { + let z = days.cast_signed() + 719_468; + let era = if z >= 0 { z } else { z - 146_096 } / 146_097; + let doe = (z - era * 146_097).cast_unsigned(); + let yoe = (doe - doe / 1460 + doe / 36524 - doe / 146_096) / 365; + let y = yoe.cast_signed() + era * 400; + let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); + let mp = (5 * doy + 2) / 153; + let d = doy - (153 * mp + 2) / 5 + 1; + let m = if mp < 10 { mp + 3 } else { mp - 9 }; + let y = if m <= 2 { y + 1 } else { y }; + (y, m, d) +} + +pub fn format_optional_epoch_ms(ms: i64) -> String { + if ms > 0 { + format_epoch_ms(ms) + } else { + "-".to_string() + } +} + +/// Format a duration as a compact elapsed time string, e.g. `(3s)` or `(1m 12s)`. +pub fn format_elapsed(d: Duration) -> String { + let secs = d.as_secs(); + if secs < 60 { + format!("({secs}s)") + } else { + let mins = secs / 60; + let rem = secs % 60; + format!("({mins}m {rem}s)") + } +} + +/// Format a total elapsed time for non-interactive mode timestamps. +pub fn format_timestamp(d: Duration) -> String { + let secs = d.as_secs_f64(); + format!("[{secs:.1}s]") +} + +/// Format a millisecond timestamp into a readable string. +pub fn format_timestamp_ms(ms: i64) -> String { + if ms <= 0 { + return "-".to_string(); + } + let secs = ms / 1000; + let mins = (secs / 60) % 60; + let hours = (secs / 3600) % 24; + let days = secs / 86400; + if days > 0 { + format!("{days}d {hours:02}:{mins:02}") + } else { + format!("{hours:02}:{mins:02}") + } +} + +pub fn truncate_status_field(value: &str, max_chars: usize) -> String { + if value.is_empty() { + return "-".to_string(); + } + let mut chars = value.chars(); + let truncated = chars.by_ref().take(max_chars).collect::(); + if chars.next().is_some() { + format!("{truncated}...") + } else { + truncated + } +} + +pub fn truncate_display(value: &str, max_width: usize) -> String { + if value.chars().count() <= max_width { + return value.to_string(); + } + + let keep = max_width.saturating_sub(3); + let mut truncated = value.chars().take(keep).collect::(); + truncated.push_str("..."); + truncated +} + +pub fn short_hash(hash: &str) -> &str { + if hash.len() >= 12 { &hash[..12] } else { hash } +} + +pub fn non_empty_or<'a>(value: &'a str, fallback: &'a str) -> &'a str { + if value.is_empty() { fallback } else { value } +} + +pub fn format_setting_value(value: Option<&SettingValue>) -> String { + let Some(value) = value.and_then(|v| v.value.as_ref()) else { + return "".to_string(); + }; + match value { + setting_value::Value::StringValue(v) => v.clone(), + setting_value::Value::BoolValue(v) => v.to_string(), + setting_value::Value::IntValue(v) => v.to_string(), + setting_value::Value::BytesValue(v) => format!("", v.len()), + } +} + +// --------------------------------------------------------------------------- +// YAML / policy display +// --------------------------------------------------------------------------- + +pub fn print_yaml_line(line: &str) { + let trimmed = line.trim_start(); + let indent = &line[..line.len() - trimmed.len()]; + + if let Some(rest) = trimmed.strip_prefix("- ") { + print!("{indent}"); + print!("{}", "- ".dimmed()); + print!("{rest}"); + println!(); + return; + } + + if let Some(colon_pos) = trimmed.find(':') { + let key = &trimmed[..colon_pos]; + let after_colon = &trimmed[colon_pos + 1..]; + + print!("{indent}"); + print!("{}", key.dimmed()); + print!("{}", ":".dimmed()); + + if after_colon.is_empty() { + // Key with nested content (no value on this line) + } else if let Some(value) = after_colon.strip_prefix(' ') { + print!(" {value}"); + } else { + print!("{after_colon}"); + } + println!(); + return; + } + + println!("{line}"); +} + +/// Print sandbox policy as YAML with dimmed keys. +pub fn print_sandbox_policy(policy: &SandboxPolicy) { + println!("{}", "Policy:".cyan().bold()); + println!(); + if let Ok(yaml_str) = openshell_policy::serialize_sandbox_policy(policy) { + for line in yaml_str.lines() { + if line == "---" { + continue; + } + print!(" "); + print_yaml_line(line); + } + } +} + +pub fn print_policy_merge_warnings(warnings: &[openshell_policy::PolicyMergeWarning]) { + for warning in warnings { + eprintln!("{} {}", "!".yellow().bold(), warning); + } +} + +// --------------------------------------------------------------------------- +// Provisioning display +// --------------------------------------------------------------------------- + +/// Known provisioning steps derived from Kubernetes events and sandbox lifecycle. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum ProvisioningStep { + /// Sandbox CRD created, waiting for pod to be scheduled. + RequestingSandbox, + /// Pulling the sandbox container image. + PullingSandboxImage, + /// Container is starting up. + StartingSandbox, +} + +impl ProvisioningStep { + /// Human-readable label for a completed step. + pub fn completed_label(self) -> &'static str { + match self { + Self::RequestingSandbox => "Sandbox allocated", + Self::PullingSandboxImage => "Image pulled", + Self::StartingSandbox => "Sandbox ready", + } + } + + /// Human-readable label for an in-progress step (shown on the spinner). + pub fn active_label(self) -> &'static str { + match self { + Self::RequestingSandbox => "Requesting sandbox...", + Self::PullingSandboxImage => "Pulling image...", + Self::StartingSandbox => "Starting sandbox...", + } + } +} + +/// Live-updating display showing a provisioning step checklist with spinner. +/// +/// Completed steps are printed as static `✓ Step` lines. The current +/// in-progress step is shown on a spinner with elapsed time. +pub struct ProvisioningDisplay { + mp: MultiProgress, + spinner: ProgressBar, + /// Blank line below the spinner so progress doesn't sit flush against + /// the bottom of the terminal. + spacer: ProgressBar, + /// Steps that have been completed, in order. + completed_steps: Vec, + /// Progress bars for completed steps (so they can be cleared). + completed_bars: Vec, + /// The currently active step label (shown on the spinner). + active_label: String, + /// Detail text shown next to the active step (e.g. image name). + active_detail: String, + /// When the current active step started (for elapsed time). + step_start: Instant, +} + +impl ProvisioningDisplay { + pub fn new() -> Self { + let mp = MultiProgress::new(); + + let spinner = mp.add(ProgressBar::new_spinner()); + spinner.set_style( + ProgressStyle::with_template("{spinner:.cyan} {msg} ({elapsed})") + .unwrap_or_else(|_| ProgressStyle::default_spinner()), + ); + spinner.enable_steady_tick(Duration::from_millis(120)); + + let spacer = mp.add(ProgressBar::new(0)); + spacer.set_style( + ProgressStyle::with_template("{msg}").unwrap_or_else(|_| ProgressStyle::default_bar()), + ); + spacer.set_message(""); + + let now = Instant::now(); + Self { + mp, + spinner, + spacer, + completed_steps: Vec::new(), + completed_bars: Vec::new(), + active_label: ProvisioningStep::RequestingSandbox + .active_label() + .to_string(), + active_detail: String::new(), + step_start: now, + } + } + + /// Record a completed provisioning step with a custom label. + pub fn complete_step_with_label(&mut self, step: ProvisioningStep, label: &str) { + if self.completed_steps.contains(&step) { + return; + } + self.completed_steps.push(step); + + let elapsed = self.step_start.elapsed(); + let elapsed_str = format_elapsed(elapsed); + + let bar = self.mp.insert_before(&self.spinner, ProgressBar::new(0)); + bar.set_style( + ProgressStyle::with_template("{msg}").unwrap_or_else(|_| ProgressStyle::default_bar()), + ); + bar.set_message(format!( + "{} {} {}", + "\u{2713}".green().bold(), + label, + elapsed_str.dimmed() + )); + bar.finish(); + self.completed_bars.push(bar); + + self.step_start = Instant::now(); + self.spinner.reset_elapsed(); + self.active_detail.clear(); + } + + /// Set the active (in-progress) step shown on the spinner. + pub fn set_active(&mut self, label: &str) { + self.active_label = label.to_string(); + self.active_detail.clear(); + self.spinner.reset_elapsed(); + self.step_start = Instant::now(); + self.update_spinner(); + } + + /// Set the active step from a known provisioning step enum. + pub fn set_active_step(&mut self, step: ProvisioningStep) { + self.set_active(step.active_label()); + } + + /// Set detail text shown alongside the active step (e.g. image name). + pub fn set_active_detail(&mut self, detail: &str) { + self.active_detail = detail.to_string(); + self.update_spinner(); + } + + fn update_spinner(&self) { + let msg = if self.active_detail.is_empty() { + self.active_label.clone() + } else { + format!("{} {}", self.active_label, self.active_detail.dimmed()) + }; + self.spinner.set_message(msg); + } + + /// Finish with an error message shown on the last step line. + pub fn finish_error(&self, msg: &str) { + let _ = self + .mp + .println(format!("{} {}", "\u{2717}".red().bold(), msg.red())); + self.spinner.finish_and_clear(); + } + + /// Print a line above the progress bars (for static header content). + pub fn println(&self, msg: &str) { + let _ = self.mp.println(msg); + } + + /// Clear all progress output (spinner, spacer, and completed step lines). + pub fn clear(&self) { + self.spacer.finish_and_clear(); + self.spinner.finish_and_clear(); + for bar in &self.completed_bars { + bar.finish_and_clear(); + } + } +} + +// --------------------------------------------------------------------------- +// Provisioning progress event handling +// --------------------------------------------------------------------------- + +pub fn progress_step_from_metadata(value: &str) -> Option { + match value { + PROGRESS_STEP_REQUESTING_SANDBOX => Some(ProvisioningStep::RequestingSandbox), + PROGRESS_STEP_PULLING_IMAGE => Some(ProvisioningStep::PullingSandboxImage), + PROGRESS_STEP_STARTING_SANDBOX => Some(ProvisioningStep::StartingSandbox), + _ => None, + } +} + +pub fn noninteractive_active_label(step: ProvisioningStep) -> String { + step.active_label().trim_end_matches('.').to_string() +} + +pub fn handle_platform_progress_event( + event: &PlatformEvent, + display: &mut Option, + provision_start: Instant, +) -> bool { + let completed_step = event + .metadata + .get(PROGRESS_COMPLETE_STEP_KEY) + .and_then(|step| progress_step_from_metadata(step)); + let active_step = event + .metadata + .get(PROGRESS_ACTIVE_STEP_KEY) + .and_then(|step| progress_step_from_metadata(step)); + let active_detail = event + .metadata + .get(PROGRESS_ACTIVE_DETAIL_KEY) + .filter(|detail| !detail.is_empty()); + + let handled = completed_step.is_some() || active_step.is_some() || active_detail.is_some(); + if !handled { + return false; + } + + if let Some(step) = completed_step { + let label = event + .metadata + .get(PROGRESS_COMPLETE_LABEL_KEY) + .map_or_else(|| step.completed_label(), String::as_str); + if let Some(d) = display.as_mut() { + d.complete_step_with_label(step, label); + } else { + let ts = format_timestamp(provision_start.elapsed()); + println!("{} {}", ts.dimmed(), label); + } + } + + if let Some(step) = active_step + && let Some(d) = display.as_mut() + { + d.set_active_step(step); + } + + if let Some(detail) = active_detail { + if let Some(d) = display.as_mut() { + d.set_active_detail(detail); + } else { + let ts = format_timestamp(provision_start.elapsed()); + if let Some(step) = active_step { + println!( + "{} {} {}", + ts.dimmed(), + noninteractive_active_label(step), + detail + ); + } else { + println!("{} {}", ts.dimmed(), detail); + } + } + } + + true +} + +pub fn is_provisioning_progress_event(event: &PlatformEvent) -> bool { + if event.metadata.contains_key(PROGRESS_COMPLETE_STEP_KEY) + || event.metadata.contains_key(PROGRESS_ACTIVE_STEP_KEY) + || event.metadata.contains_key(PROGRESS_ACTIVE_DETAIL_KEY) + { + return true; + } + + event.source == "vm" + && matches!( + event.reason.as_str(), + "PullingLayer" + | "ResolvingImage" + | "AuthenticatingRegistry" + | "FetchingManifest" + | "CacheHit" + | "CacheMiss" + | "WaitingForImageCacheLock" + | "ExportingRootfs" + | "PreparingRootfs" + | "CreatingRootDisk" + | "PreparingOverlay" + | "Started" + ) +} + +// --------------------------------------------------------------------------- +// Sandbox header +// --------------------------------------------------------------------------- + +pub fn print_sandbox_header( + sandbox: &openshell_core::proto::Sandbox, + display: Option<&ProvisioningDisplay>, +) { + use openshell_core::ObjectName; + + let lines = [ + String::new(), + format!( + "{} {}", + "Created sandbox:".cyan().bold(), + sandbox.object_name().bold() + ), + String::new(), + ]; + match display { + Some(d) => { + for line in lines { + d.println(&line); + } + } + None => { + for line in lines { + println!("{line}"); + } + } + } +} + +// --------------------------------------------------------------------------- +// Sandbox provisioning helpers +// --------------------------------------------------------------------------- + +pub fn ready_false_condition_message( + status: Option<&openshell_core::proto::SandboxStatus>, +) -> Option { + let condition = status?.conditions.iter().find(|condition| { + condition.r#type == "Ready" && condition.status.eq_ignore_ascii_case("false") + })?; + + if condition.message.is_empty() { + if condition.reason.is_empty() { + None + } else { + Some(condition.reason.clone()) + } + } else if condition.reason.is_empty() { + Some(condition.message.clone()) + } else { + Some(format!("{}: {}", condition.reason, condition.message)) + } +} + +pub fn provisioning_timeout_message( + timeout_secs: u64, + resource_requirements: Option<&openshell_core::proto::ResourceRequirements>, + condition_message: Option<&str>, +) -> String { + let mut message = format!("sandbox provisioning timed out after {timeout_secs}s"); + + if let Some(condition_message) = condition_message.filter(|msg| !msg.is_empty()) { + message.push_str(". Last reported status: "); + message.push_str(condition_message); + } + + if resource_requirements.is_some_and(|requirements| requirements.gpu.is_some()) { + message.push_str( + ". Hint: this may be because the available GPU is already in use by another sandbox.", + ); + } + + message +} + +// --------------------------------------------------------------------------- +// Settings helpers +// --------------------------------------------------------------------------- + +pub fn parse_cli_setting_value(key: &str, raw_value: &str) -> Result { + let setting = settings::setting_for_key(key).ok_or_else(|| { + miette::miette!( + "unknown setting key '{}'. Allowed keys: {}", + key, + settings::registered_keys_csv() + ) + })?; + + let value = match setting.kind { + SettingValueKind::String => { + setting + .validate_string_value(raw_value) + .map_err(|allowed| { + miette::miette!( + "invalid value '{}' for key '{}'; expected one of: {}", + raw_value, + key, + allowed.join(", ") + ) + })?; + setting_value::Value::StringValue(raw_value.to_string()) + } + SettingValueKind::Int => { + let parsed = raw_value.trim().parse::().map_err(|_| { + miette::miette!( + "invalid int value '{}' for key '{}'; expected base-10 integer", + raw_value, + key + ) + })?; + setting_value::Value::IntValue(parsed) + } + SettingValueKind::Bool => { + let parsed = settings::parse_bool_like(raw_value).ok_or_else(|| { + miette::miette!( + "invalid bool value '{}' for key '{}'; expected one of: true,false,yes,no,1,0", + raw_value, + key + ) + })?; + setting_value::Value::BoolValue(parsed) + } + }; + + Ok(SettingValue { value: Some(value) }) +} + +pub fn confirm_global_setting_takeover(key: &str, yes: bool) -> Result<()> { + if yes { + return Ok(()); + } + + if !std::io::stdin().is_terminal() || !std::io::stdout().is_terminal() { + return Err(miette::miette!( + "global setting updates require confirmation; pass --yes in non-interactive mode" + )); + } + + let proceed = Confirm::with_theme(&ColorfulTheme::default()) + .with_prompt(format!( + "Setting '{key}' globally will disable sandbox-level management for this key. Continue?" + )) + .default(false) + .interact() + .into_diagnostic()?; + + if !proceed { + return Err(miette::miette!("aborted by user")); + } + + Ok(()) +} + +pub fn confirm_global_setting_delete(key: &str, yes: bool) -> Result<()> { + if yes { + return Ok(()); + } + + if !std::io::stdin().is_terminal() || !std::io::stdout().is_terminal() { + return Err(miette::miette!( + "global setting deletes require confirmation; pass --yes in non-interactive mode" + )); + } + + let proceed = Confirm::with_theme(&ColorfulTheme::default()) + .with_prompt(format!( + "Deleting global setting '{key}' re-enables sandbox-level management for this key. Continue?" + )) + .default(false) + .interact() + .into_diagnostic()?; + + if !proceed { + return Err(miette::miette!("aborted by user")); + } + + Ok(()) +} + +/// Parse a duration string like "5m", "1h", "30s" into milliseconds. +pub fn parse_duration_to_ms(s: &str) -> Result { + let s = s.trim(); + if s.is_empty() { + return Err(miette::miette!("empty duration string")); + } + let (num_str, unit) = s.split_at(s.len() - 1); + let num: i64 = num_str + .parse() + .map_err(|_| miette::miette!("invalid duration: {s} (expected e.g. 5m, 1h, 30s)"))?; + let multiplier = match unit { + "s" => 1_000, + "m" => 60_000, + "h" => 3_600_000, + _ => { + return Err(miette::miette!( + "unknown duration unit: {unit} (use s, m, or h)" + )); + } + }; + Ok(num * multiplier) +} + +// --------------------------------------------------------------------------- +// Parsing utilities +// --------------------------------------------------------------------------- + +pub fn parse_key_value_pairs(items: &[String], flag: &str) -> Result> { + let mut map = HashMap::new(); + + for item in items { + let Some((key, value)) = item.split_once('=') else { + return Err(miette::miette!("{flag} expects KEY=VALUE, got '{item}'")); + }; + + let key = key.trim(); + if key.is_empty() { + return Err(miette::miette!("{flag} key cannot be empty")); + } + + map.insert(key.to_string(), value.to_string()); + } + + Ok(map) +} + +pub fn parse_env_pairs(items: &[String]) -> Result> { + let map = parse_key_value_pairs(items, "--env")?; + for key in map.keys() { + if !is_valid_env_name(key) { + return Err(miette::miette!( + "--env key must match [A-Za-z_][A-Za-z0-9_]*; got '{key}'" + )); + } + if key.starts_with("OPENSHELL_") { + return Err(miette::miette!( + "--env keys starting with OPENSHELL_ are reserved; got '{key}'" + )); + } + } + Ok(map) +} + +/// Resolve `--secret-material-env KEY[=ENVVAR]` values from the CLI process +/// environment (`ENVVAR` defaults to `KEY`) so secrets never transit argv. +pub fn parse_secret_material_env_pairs(items: &[String]) -> Result> { + let mut map = HashMap::new(); + + for item in items { + let (key, env_name) = match item.split_once('=') { + Some((key, env_name)) => (key.trim(), env_name.trim()), + None => (item.trim(), item.trim()), + }; + if key.is_empty() { + return Err(miette::miette!("--secret-material-env key cannot be empty")); + } + if env_name.is_empty() { + return Err(miette::miette!( + "--secret-material-env {key} names an empty environment variable" + )); + } + + let value = std::env::var(env_name).map_err(|_| { + miette::miette!( + "--secret-material-env {key} requires local env var '{env_name}' to be set to a non-empty value" + ) + })?; + if value.trim().is_empty() { + return Err(miette::miette!( + "--secret-material-env {key} requires local env var '{env_name}' to be set to a non-empty value" + )); + } + + if map.contains_key(key) { + return Err(miette::miette!( + "--secret-material-env key '{key}' supplied more than once" + )); + } + map.insert(key.to_string(), value); + } + + Ok(map) +} + +pub fn is_valid_env_name(key: &str) -> bool { + let mut bytes = key.bytes(); + let Some(first) = bytes.next() else { + return false; + }; + if !(first == b'_' || first.is_ascii_alphabetic()) { + return false; + } + bytes.all(|b| b == b'_' || b.is_ascii_alphanumeric()) +} + +pub fn parse_credential_pairs(items: &[String]) -> Result> { + let mut map = HashMap::new(); + + for item in items { + if let Some((key, value)) = item.split_once('=') { + let key = key.trim(); + if key.is_empty() { + return Err(miette::miette!("--credential key cannot be empty")); + } + map.insert(key.to_string(), value.to_string()); + continue; + } + + let key = item.trim(); + if key.is_empty() { + return Err(miette::miette!("--credential key cannot be empty")); + } + + let value = std::env::var(key).map_err(|_| { + miette::miette!( + "--credential {key} requires local env var '{key}' to be set to a non-empty value" + ) + })?; + + if value.trim().is_empty() { + return Err(miette::miette!( + "--credential {key} requires local env var '{key}' to be set to a non-empty value" + )); + } + + map.insert(key.to_string(), value); + } + + Ok(map) +} + +pub fn parse_credential_expiry_cli_value(value: &str) -> std::result::Result { + parse_credential_expiry_value(value, None).map_err(|err| err.to_string()) +} + +fn credential_expiry_value_error(key: Option<&str>, detail: &str) -> miette::Report { + key.map_or_else( + || miette::miette!("--credential-expires-at value {detail}"), + |key| miette::miette!("--credential-expires-at value for '{key}' {detail}"), + ) +} + +pub fn parse_credential_expiry_value(value: &str, key: Option<&str>) -> Result { + let value = value.trim(); + if value.is_empty() { + return Err(credential_expiry_value_error(key, "cannot be empty")); + } + + if let Ok(value_ms) = value.parse::() { + if value_ms < 0 { + return Err(credential_expiry_value_error( + key, + "must be greater than or equal to 0", + )); + } + return Ok(value_ms); + } + + let parsed = DateTime::parse_from_rfc3339(value).map_err(|_| { + credential_expiry_value_error( + key, + "must be a Unix epoch millisecond timestamp or RFC3339 timestamp", + ) + })?; + let value_ms = parsed.timestamp_millis(); + if value_ms < 0 { + return Err(credential_expiry_value_error( + key, + "must be greater than or equal to 0", + )); + } + + Ok(value_ms) +} + +pub fn parse_credential_expiry_pairs(items: &[String]) -> Result> { + let mut map = HashMap::new(); + + for item in items { + let Some((key, value)) = item.split_once('=') else { + return Err(miette::miette!( + "--credential-expires-at expects KEY=TIMESTAMP, got '{item}'" + )); + }; + let key = key.trim(); + if key.is_empty() { + return Err(miette::miette!( + "--credential-expires-at key cannot be empty" + )); + } + let value = parse_credential_expiry_value(value, Some(key))?; + map.insert(key.to_string(), value); + } + + Ok(map) +} + +// --------------------------------------------------------------------------- +// Git environment helpers +// --------------------------------------------------------------------------- + +pub fn scrub_git_env(command: &mut Command) -> &mut Command { + for key in [ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_PREFIX", + "GIT_COMMON_DIR", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + ] { + command.env_remove(key); + } + command +} diff --git a/crates/openshell-cli/src/commands/mod.rs b/crates/openshell-cli/src/commands/mod.rs new file mode 100644 index 0000000000..ab20f224db --- /dev/null +++ b/crates/openshell-cli/src/commands/mod.rs @@ -0,0 +1,4 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +pub mod common; diff --git a/crates/openshell-cli/src/lib.rs b/crates/openshell-cli/src/lib.rs index 156668951a..bc5832e839 100644 --- a/crates/openshell-cli/src/lib.rs +++ b/crates/openshell-cli/src/lib.rs @@ -9,6 +9,7 @@ pub(crate) static TEST_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); pub mod auth; +pub mod commands; pub mod completers; pub mod edge_tunnel; pub mod oidc_auth; diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 7b68487ea1..c155948c26 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -3,20 +3,34 @@ //! CLI command implementations. +pub use crate::commands::common::{ + PolicyGetView, confirm_global_setting_delete, confirm_global_setting_takeover, + is_valid_env_name, parse_credential_expiry_cli_value, parse_credential_expiry_value, + parse_duration_to_ms, parse_env_pairs, parse_key_value_pairs, parse_secret_material_env_pairs, +}; +use crate::commands::common::{ + ProvisioningDisplay, ProvisioningStep, format_epoch_ms, format_optional_epoch_ms, + format_setting_value, format_timestamp, format_timestamp_ms, handle_platform_progress_event, + is_provisioning_progress_event, non_empty_or, parse_cli_setting_value, + parse_credential_expiry_pairs, parse_credential_pairs, phase_name, print_policy_merge_warnings, + print_sandbox_header, print_sandbox_policy, provisioning_timeout_message, + ready_false_condition_message, scrub_git_env, short_hash, truncate_display, + truncate_status_field, +}; + use crate::policy_update::build_policy_update_plan; use crate::tls::{ TlsOptions, build_insecure_rustls_config, build_rustls_config, grpc_client, grpc_inference_client, require_tls_materials, }; use bytes::Bytes; -use chrono::DateTime; use dialoguer::{Confirm, Select, theme::ColorfulTheme}; use futures::StreamExt; use http_body_util::Full; use hyper::{Request, StatusCode}; use hyper_rustls::HttpsConnectorBuilder; use hyper_util::{client::legacy::Client, rt::TokioExecutor}; -use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use indicatif::{ProgressBar, ProgressStyle}; use miette::{IntoDiagnostic, Result, WrapErr, miette}; use openshell_bootstrap::{ GatewayMetadata, clear_active_gateway, clear_last_sandbox_if_matches, @@ -27,11 +41,6 @@ use openshell_bootstrap::{ use openshell_bootstrap::{ GatewayMetadataSource, ListedGateway, gateway_metadata_source, list_gateways_with_source, }; -use openshell_core::progress::{ - PROGRESS_ACTIVE_DETAIL_KEY, PROGRESS_ACTIVE_STEP_KEY, PROGRESS_COMPLETE_LABEL_KEY, - PROGRESS_COMPLETE_STEP_KEY, PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, - PROGRESS_STEP_STARTING_SANDBOX, -}; use openshell_core::proto::ProviderProfileCategory; use openshell_core::proto::{ ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, AttachSandboxProviderRequest, @@ -46,17 +55,16 @@ use openshell_core::proto::{ GpuResourceRequirements, HealthRequest, ImportProviderProfilesRequest, LintProviderProfilesRequest, ListProviderProfilesRequest, ListProvidersRequest, ListSandboxPoliciesRequest, ListSandboxProvidersRequest, ListSandboxesRequest, - ListServicesRequest, PlatformEvent, PolicySource, PolicyStatus, Provider, - ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, ProviderProfile, - ProviderProfileDiagnostic, ProviderProfileImportItem, RejectDraftChunkRequest, - ResourceRequirements, RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, - SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, ServiceEndpointResponse, - ServiceStatus, SetInferenceRouteRequest, SettingScope, SettingValue, TcpForwardFrame, - TcpForwardInit, TcpRelayTarget, UpdateConfigRequest, UpdateProviderProfilesRequest, - UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, setting_value, - tcp_forward_init, + ListServicesRequest, PolicySource, PolicyStatus, Provider, ProviderCredentialRefreshStatus, + ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileDiagnostic, + ProviderProfileImportItem, RejectDraftChunkRequest, ResourceRequirements, + RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy, + SandboxSpec, SandboxTemplate, ServiceEndpointResponse, ServiceStatus, + SetInferenceRouteRequest, SettingScope, TcpForwardFrame, TcpForwardInit, TcpRelayTarget, + UpdateConfigRequest, UpdateProviderProfilesRequest, UpdateProviderRequest, WatchSandboxRequest, + exec_sandbox_event, setting_value, tcp_forward_init, }; -use openshell_core::settings::{self, SettingValueKind}; +use openshell_core::settings; use openshell_core::{ObjectId, ObjectName, ObjectWorkspace}; use openshell_providers::{ ProviderRegistry, ProviderTypeProfile, RealDiscoveryContext, detect_provider_from_command, @@ -82,27 +90,6 @@ pub use openshell_core::forward::{ ForwardSpec, find_forward_by_port, list_forwards, stop_forward, stop_forwards_for_sandbox, }; -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum PolicyGetView { - Metadata, - Base, - Full, -} - -impl PolicyGetView { - pub fn from_flags(base: bool, full: bool) -> Self { - match (base, full) { - (true, _) => Self::Base, - (false, true) => Self::Full, - (false, false) => Self::Metadata, - } - } - - fn includes_policy(self) -> bool { - matches!(self, Self::Base | Self::Full) - } -} - #[derive(Debug, PartialEq, Eq)] enum SandboxUploadPlan { GitAware { @@ -113,419 +100,6 @@ enum SandboxUploadPlan { GitFilteredEmpty, } -/// Convert a sandbox phase integer to a human-readable string. -fn phase_name(phase: i32) -> &'static str { - match SandboxPhase::try_from(phase) { - Ok(SandboxPhase::Unspecified) => "Unspecified", - Ok(SandboxPhase::Provisioning) => "Provisioning", - Ok(SandboxPhase::Ready) => "Ready", - Ok(SandboxPhase::Error) => "Error", - Ok(SandboxPhase::Deleting) => "Deleting", - Ok(SandboxPhase::Unknown) | Err(_) => "Unknown", - } -} - -fn ready_false_condition_message( - status: Option<&openshell_core::proto::SandboxStatus>, -) -> Option { - let condition = status?.conditions.iter().find(|condition| { - condition.r#type == "Ready" && condition.status.eq_ignore_ascii_case("false") - })?; - - if condition.message.is_empty() { - if condition.reason.is_empty() { - None - } else { - Some(condition.reason.clone()) - } - } else if condition.reason.is_empty() { - Some(condition.message.clone()) - } else { - Some(format!("{}: {}", condition.reason, condition.message)) - } -} - -fn provisioning_timeout_message( - timeout_secs: u64, - resource_requirements: Option<&ResourceRequirements>, - condition_message: Option<&str>, -) -> String { - let mut message = format!("sandbox provisioning timed out after {timeout_secs}s"); - - if let Some(condition_message) = condition_message.filter(|msg| !msg.is_empty()) { - message.push_str(". Last reported status: "); - message.push_str(condition_message); - } - - if resource_requirements.is_some_and(|requirements| requirements.gpu.is_some()) { - message.push_str( - ". Hint: this may be because the available GPU is already in use by another sandbox.", - ); - } - - message -} - -/// Format milliseconds since Unix epoch as a `YYYY-MM-DD HH:MM:SS` UTC string. -fn format_epoch_ms(ms: i64) -> String { - use std::time::UNIX_EPOCH; - - let Ok(ms_u64) = u64::try_from(ms) else { - return "-".to_string(); - }; - let Ok(time) = UNIX_EPOCH - .checked_add(Duration::from_millis(ms_u64)) - .ok_or(()) - else { - return "-".to_string(); - }; - let Ok(dur) = time.duration_since(UNIX_EPOCH) else { - return "-".to_string(); - }; - - let secs = dur.as_secs(); - let days = secs / 86400; - let time_of_day = secs % 86400; - let hours = time_of_day / 3600; - let minutes = (time_of_day % 3600) / 60; - let seconds = time_of_day % 60; - - // Convert days since epoch to year-month-day using a basic civil calendar algorithm. - let (y, m, d) = civil_from_days(days); - format!("{y:04}-{m:02}-{d:02} {hours:02}:{minutes:02}:{seconds:02}") -} - -/// Convert days since 1970-01-01 to (year, month, day). -/// Algorithm from Howard Hinnant's `chrono`-compatible date library. -fn civil_from_days(days: u64) -> (i64, u64, u64) { - let z = days.cast_signed() + 719_468; - let era = if z >= 0 { z } else { z - 146_096 } / 146_097; - let doe = (z - era * 146_097).cast_unsigned(); - let yoe = (doe - doe / 1460 + doe / 36524 - doe / 146_096) / 365; - let y = yoe.cast_signed() + era * 400; - let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); - let mp = (5 * doy + 2) / 153; - let d = doy - (153 * mp + 2) / 5 + 1; - let m = if mp < 10 { mp + 3 } else { mp - 9 }; - let y = if m <= 2 { y + 1 } else { y }; - (y, m, d) -} - -/// Known provisioning steps derived from Kubernetes events and sandbox lifecycle. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] -enum ProvisioningStep { - /// Sandbox CRD created, waiting for pod to be scheduled. - RequestingSandbox, - /// Pulling the sandbox container image. - PullingSandboxImage, - /// Container is starting up. - StartingSandbox, -} - -impl ProvisioningStep { - /// Human-readable label for a completed step. - fn completed_label(self) -> &'static str { - match self { - Self::RequestingSandbox => "Sandbox allocated", - Self::PullingSandboxImage => "Image pulled", - Self::StartingSandbox => "Sandbox ready", - } - } - - /// Human-readable label for an in-progress step (shown on the spinner). - fn active_label(self) -> &'static str { - match self { - Self::RequestingSandbox => "Requesting sandbox...", - Self::PullingSandboxImage => "Pulling image...", - Self::StartingSandbox => "Starting sandbox...", - } - } -} - -/// Live-updating display showing a provisioning step checklist with spinner. -/// -/// Completed steps are printed as static `✓ Step` lines. The current -/// in-progress step is shown on a spinner with elapsed time. -struct ProvisioningDisplay { - mp: MultiProgress, - spinner: ProgressBar, - /// Blank line below the spinner so progress doesn't sit flush against - /// the bottom of the terminal. - spacer: ProgressBar, - /// Steps that have been completed, in order. - completed_steps: Vec, - /// Progress bars for completed steps (so they can be cleared). - completed_bars: Vec, - /// The currently active step label (shown on the spinner). - active_label: String, - /// Detail text shown next to the active step (e.g. image name). - active_detail: String, - /// When the current active step started (for elapsed time). - step_start: Instant, -} - -impl ProvisioningDisplay { - fn new() -> Self { - let mp = MultiProgress::new(); - - let spinner = mp.add(ProgressBar::new_spinner()); - spinner.set_style( - ProgressStyle::with_template("{spinner:.cyan} {msg} ({elapsed})") - .unwrap_or_else(|_| ProgressStyle::default_spinner()), - ); - spinner.enable_steady_tick(Duration::from_millis(120)); - - // Always keep a blank line below the spinner so the progress area - // doesn't sit flush against the bottom of the terminal. - let spacer = mp.add(ProgressBar::new(0)); - spacer.set_style( - ProgressStyle::with_template("{msg}").unwrap_or_else(|_| ProgressStyle::default_bar()), - ); - spacer.set_message(""); - - let now = Instant::now(); - Self { - mp, - spinner, - spacer, - completed_steps: Vec::new(), - completed_bars: Vec::new(), - active_label: ProvisioningStep::RequestingSandbox - .active_label() - .to_string(), - active_detail: String::new(), - step_start: now, - } - } - - /// Record a completed provisioning step with a custom label. - fn complete_step_with_label(&mut self, step: ProvisioningStep, label: &str) { - // Don't duplicate steps we've already printed. - if self.completed_steps.contains(&step) { - return; - } - self.completed_steps.push(step); - - let elapsed = self.step_start.elapsed(); - let elapsed_str = format_elapsed(elapsed); - - // Use a progress bar instead of println so we can clear it later. - let bar = self.mp.insert_before(&self.spinner, ProgressBar::new(0)); - bar.set_style( - ProgressStyle::with_template("{msg}").unwrap_or_else(|_| ProgressStyle::default_bar()), - ); - bar.set_message(format!( - "{} {} {}", - "\u{2713}".green().bold(), - label, - elapsed_str.dimmed() - )); - bar.finish(); - self.completed_bars.push(bar); - - // Reset step timer for the next step. - self.step_start = Instant::now(); - self.spinner.reset_elapsed(); - self.active_detail.clear(); - } - - /// Set the active (in-progress) step shown on the spinner. - fn set_active(&mut self, label: &str) { - self.active_label = label.to_string(); - self.active_detail.clear(); - // Reset the spinner's elapsed time for the new step. - self.spinner.reset_elapsed(); - self.step_start = Instant::now(); - self.update_spinner(); - } - - /// Set the active step from a known provisioning step enum. - fn set_active_step(&mut self, step: ProvisioningStep) { - self.set_active(step.active_label()); - } - - /// Set detail text shown alongside the active step (e.g. image name). - fn set_active_detail(&mut self, detail: &str) { - self.active_detail = detail.to_string(); - self.update_spinner(); - } - - fn update_spinner(&self) { - let msg = if self.active_detail.is_empty() { - self.active_label.clone() - } else { - format!("{} {}", self.active_label, self.active_detail.dimmed()) - }; - self.spinner.set_message(msg); - } - - /// Finish with an error message shown on the last step line. - fn finish_error(&self, msg: &str) { - let _ = self - .mp - .println(format!("{} {}", "\u{2717}".red().bold(), msg.red())); - self.spinner.finish_and_clear(); - } - - /// Print a line above the progress bars (for static header content). - fn println(&self, msg: &str) { - let _ = self.mp.println(msg); - } - - /// Clear all progress output (spinner, spacer, and completed step lines). - fn clear(&self) { - self.spacer.finish_and_clear(); - self.spinner.finish_and_clear(); - for bar in &self.completed_bars { - bar.finish_and_clear(); - } - } -} - -/// Format a duration as a compact elapsed time string, e.g. `(3s)` or `(1m 12s)`. -fn format_elapsed(d: Duration) -> String { - let secs = d.as_secs(); - if secs < 60 { - format!("({secs}s)") - } else { - let mins = secs / 60; - let rem = secs % 60; - format!("({mins}m {rem}s)") - } -} - -/// Format a total elapsed time for non-interactive mode timestamps. -fn format_timestamp(d: Duration) -> String { - let secs = d.as_secs_f64(); - format!("[{secs:.1}s]") -} - -fn progress_step_from_metadata(value: &str) -> Option { - match value { - PROGRESS_STEP_REQUESTING_SANDBOX => Some(ProvisioningStep::RequestingSandbox), - PROGRESS_STEP_PULLING_IMAGE => Some(ProvisioningStep::PullingSandboxImage), - PROGRESS_STEP_STARTING_SANDBOX => Some(ProvisioningStep::StartingSandbox), - _ => None, - } -} - -fn noninteractive_active_label(step: ProvisioningStep) -> String { - step.active_label().trim_end_matches('.').to_string() -} - -fn handle_platform_progress_event( - event: &PlatformEvent, - display: &mut Option, - provision_start: Instant, -) -> bool { - let completed_step = event - .metadata - .get(PROGRESS_COMPLETE_STEP_KEY) - .and_then(|step| progress_step_from_metadata(step)); - let active_step = event - .metadata - .get(PROGRESS_ACTIVE_STEP_KEY) - .and_then(|step| progress_step_from_metadata(step)); - let active_detail = event - .metadata - .get(PROGRESS_ACTIVE_DETAIL_KEY) - .filter(|detail| !detail.is_empty()); - - let handled = completed_step.is_some() || active_step.is_some() || active_detail.is_some(); - if !handled { - return false; - } - - if let Some(step) = completed_step { - let label = event - .metadata - .get(PROGRESS_COMPLETE_LABEL_KEY) - .map_or_else(|| step.completed_label(), String::as_str); - if let Some(d) = display.as_mut() { - d.complete_step_with_label(step, label); - } else { - let ts = format_timestamp(provision_start.elapsed()); - println!("{} {}", ts.dimmed(), label); - } - } - - if let Some(step) = active_step - && let Some(d) = display.as_mut() - { - d.set_active_step(step); - } - - if let Some(detail) = active_detail { - if let Some(d) = display.as_mut() { - d.set_active_detail(detail); - } else { - let ts = format_timestamp(provision_start.elapsed()); - if let Some(step) = active_step { - println!( - "{} {} {}", - ts.dimmed(), - noninteractive_active_label(step), - detail - ); - } else { - println!("{} {}", ts.dimmed(), detail); - } - } - } - - true -} - -fn is_provisioning_progress_event(event: &PlatformEvent) -> bool { - if event.metadata.contains_key(PROGRESS_COMPLETE_STEP_KEY) - || event.metadata.contains_key(PROGRESS_ACTIVE_STEP_KEY) - || event.metadata.contains_key(PROGRESS_ACTIVE_DETAIL_KEY) - { - return true; - } - - event.source == "vm" - && matches!( - event.reason.as_str(), - "PullingLayer" - | "ResolvingImage" - | "AuthenticatingRegistry" - | "FetchingManifest" - | "CacheHit" - | "CacheMiss" - | "WaitingForImageCacheLock" - | "ExportingRootfs" - | "PreparingRootfs" - | "CreatingRootDisk" - | "PreparingOverlay" - | "Started" - ) -} - -fn print_sandbox_header(sandbox: &Sandbox, display: Option<&ProvisioningDisplay>) { - let lines = [ - String::new(), - format!( - "{} {}", - "Created sandbox:".cyan().bold(), - sandbox.object_name().bold() - ), - String::new(), - ]; - match display { - Some(d) => { - for line in lines { - d.println(&line); - } - } - None => { - for line in lines { - println!("{line}"); - } - } - } -} - #[derive(Debug, Clone)] struct GatewayInfoView { gateway: String, @@ -3451,61 +3025,6 @@ async fn sandbox_exec_interactive_grpc( } /// Print a single YAML line with dimmed keys and regular values. -fn print_yaml_line(line: &str) { - // Find leading whitespace - let trimmed = line.trim_start(); - let indent = &line[..line.len() - trimmed.len()]; - - // Handle list items - if let Some(rest) = trimmed.strip_prefix("- ") { - print!("{indent}"); - print!("{}", "- ".dimmed()); - print!("{rest}"); - println!(); - return; - } - - // Handle key: value pairs - if let Some(colon_pos) = trimmed.find(':') { - let key = &trimmed[..colon_pos]; - let after_colon = &trimmed[colon_pos + 1..]; - - print!("{indent}"); - print!("{}", key.dimmed()); - print!("{}", ":".dimmed()); - - if after_colon.is_empty() { - // Key with nested content (no value on this line) - } else if let Some(value) = after_colon.strip_prefix(' ') { - // Key: value - print!(" {value}"); - } else { - // Shouldn't happen in valid YAML, but handle it - print!("{after_colon}"); - } - println!(); - return; - } - - // Plain line (shouldn't happen often in YAML) - println!("{line}"); -} - -/// Print sandbox policy as YAML with dimmed keys. -fn print_sandbox_policy(policy: &SandboxPolicy) { - println!("{}", "Policy:".cyan().bold()); - println!(); - if let Ok(yaml_str) = openshell_policy::serialize_sandbox_policy(policy) { - // Indent the YAML output and skip the initial "---" line - for line in yaml_str.lines() { - if line == "---" { - continue; - } - print!(" "); - print_yaml_line(line); - } - } -} /// List sandboxes. #[allow(clippy::too_many_arguments)] @@ -4228,196 +3747,6 @@ async fn auto_create_provider( Ok(()) } -pub fn parse_key_value_pairs(items: &[String], flag: &str) -> Result> { - let mut map = HashMap::new(); - - for item in items { - let Some((key, value)) = item.split_once('=') else { - return Err(miette::miette!("{flag} expects KEY=VALUE, got '{item}'")); - }; - - let key = key.trim(); - if key.is_empty() { - return Err(miette::miette!("{flag} key cannot be empty")); - } - - map.insert(key.to_string(), value.to_string()); - } - - Ok(map) -} - -pub fn parse_env_pairs(items: &[String]) -> Result> { - let map = parse_key_value_pairs(items, "--env")?; - for key in map.keys() { - if !is_valid_env_name(key) { - return Err(miette::miette!( - "--env key must match [A-Za-z_][A-Za-z0-9_]*; got '{key}'" - )); - } - if key.starts_with("OPENSHELL_") { - return Err(miette::miette!( - "--env keys starting with OPENSHELL_ are reserved; got '{key}'" - )); - } - } - Ok(map) -} - -/// Resolve `--secret-material-env KEY[=ENVVAR]` values from the CLI process -/// environment (`ENVVAR` defaults to `KEY`) so secrets never transit argv. -pub fn parse_secret_material_env_pairs(items: &[String]) -> Result> { - let mut map = HashMap::new(); - - for item in items { - let (key, env_name) = match item.split_once('=') { - Some((key, env_name)) => (key.trim(), env_name.trim()), - None => (item.trim(), item.trim()), - }; - if key.is_empty() { - return Err(miette::miette!("--secret-material-env key cannot be empty")); - } - if env_name.is_empty() { - return Err(miette::miette!( - "--secret-material-env {key} names an empty environment variable" - )); - } - - let value = std::env::var(env_name).map_err(|_| { - miette::miette!( - "--secret-material-env {key} requires local env var '{env_name}' to be set to a non-empty value" - ) - })?; - if value.trim().is_empty() { - return Err(miette::miette!( - "--secret-material-env {key} requires local env var '{env_name}' to be set to a non-empty value" - )); - } - - if map.contains_key(key) { - return Err(miette::miette!( - "--secret-material-env key '{key}' supplied more than once" - )); - } - map.insert(key.to_string(), value); - } - - Ok(map) -} - -fn is_valid_env_name(key: &str) -> bool { - let mut bytes = key.bytes(); - let Some(first) = bytes.next() else { - return false; - }; - if !(first == b'_' || first.is_ascii_alphabetic()) { - return false; - } - bytes.all(|b| b == b'_' || b.is_ascii_alphanumeric()) -} - -fn parse_credential_pairs(items: &[String]) -> Result> { - let mut map = HashMap::new(); - - for item in items { - if let Some((key, value)) = item.split_once('=') { - let key = key.trim(); - if key.is_empty() { - return Err(miette::miette!("--credential key cannot be empty")); - } - map.insert(key.to_string(), value.to_string()); - continue; - } - - let key = item.trim(); - if key.is_empty() { - return Err(miette::miette!("--credential key cannot be empty")); - } - - let value = std::env::var(key).map_err(|_| { - miette::miette!( - "--credential {key} requires local env var '{key}' to be set to a non-empty value" - ) - })?; - - if value.trim().is_empty() { - return Err(miette::miette!( - "--credential {key} requires local env var '{key}' to be set to a non-empty value" - )); - } - - map.insert(key.to_string(), value); - } - - Ok(map) -} - -pub fn parse_credential_expiry_cli_value(value: &str) -> std::result::Result { - parse_credential_expiry_value(value, None).map_err(|err| err.to_string()) -} - -fn credential_expiry_value_error(key: Option<&str>, detail: &str) -> miette::Report { - key.map_or_else( - || miette::miette!("--credential-expires-at value {detail}"), - |key| miette::miette!("--credential-expires-at value for '{key}' {detail}"), - ) -} - -fn parse_credential_expiry_value(value: &str, key: Option<&str>) -> Result { - let value = value.trim(); - if value.is_empty() { - return Err(credential_expiry_value_error(key, "cannot be empty")); - } - - if let Ok(value_ms) = value.parse::() { - if value_ms < 0 { - return Err(credential_expiry_value_error( - key, - "must be greater than or equal to 0", - )); - } - return Ok(value_ms); - } - - let parsed = DateTime::parse_from_rfc3339(value).map_err(|_| { - credential_expiry_value_error( - key, - "must be a Unix epoch millisecond timestamp or RFC3339 timestamp", - ) - })?; - let value_ms = parsed.timestamp_millis(); - if value_ms < 0 { - return Err(credential_expiry_value_error( - key, - "must be greater than or equal to 0", - )); - } - - Ok(value_ms) -} - -fn parse_credential_expiry_pairs(items: &[String]) -> Result> { - let mut map = HashMap::new(); - - for item in items { - let Some((key, value)) = item.split_once('=') else { - return Err(miette::miette!( - "--credential-expires-at expects KEY=TIMESTAMP, got '{item}'" - )); - }; - let key = key.trim(); - if key.is_empty() { - return Err(miette::miette!( - "--credential-expires-at key cannot be empty" - )); - } - let value = parse_credential_expiry_value(value, Some(key))?; - map.insert(key.to_string(), value); - } - - Ok(map) -} - pub async fn service_expose( server: &str, sandbox: &str, @@ -5903,27 +5232,6 @@ fn refresh_status_row(status: &ProviderCredentialRefreshStatus) -> String { ) } -fn format_optional_epoch_ms(ms: i64) -> String { - if ms > 0 { - format_epoch_ms(ms) - } else { - "-".to_string() - } -} - -fn truncate_status_field(value: &str, max_chars: usize) -> String { - if value.is_empty() { - return "-".to_string(); - } - let mut chars = value.chars(); - let truncated = chars.by_ref().take(max_chars).collect::(); - if chars.next().is_some() { - format!("{truncated}...") - } else { - truncated - } -} - fn provider_refresh_strategy_name(strategy: ProviderCredentialRefreshStrategy) -> &'static str { match strategy { ProviderCredentialRefreshStrategy::Static => "static", @@ -6161,17 +5469,6 @@ fn print_provider_type_row( ); } -fn truncate_display(value: &str, max_width: usize) -> String { - if value.chars().count() <= max_width { - return value.to_string(); - } - - let keep = max_width.saturating_sub(3); - let mut truncated = value.chars().take(keep).collect::(); - truncated.push_str("..."); - truncated -} - #[allow(clippy::too_many_arguments)] pub async fn provider_update( server: &str, @@ -7119,174 +6416,10 @@ pub async fn sandbox_upload( Ok(()) } -fn scrub_git_env(command: &mut Command) -> &mut Command { - for key in [ - "GIT_DIR", - "GIT_WORK_TREE", - "GIT_INDEX_FILE", - "GIT_PREFIX", - "GIT_COMMON_DIR", - "GIT_OBJECT_DIRECTORY", - "GIT_ALTERNATE_OBJECT_DIRECTORIES", - ] { - command.env_remove(key); - } - command -} - // --------------------------------------------------------------------------- // Sandbox policy commands // --------------------------------------------------------------------------- -/// Parse a duration string like "5m", "1h", "30s" into milliseconds. -fn parse_duration_to_ms(s: &str) -> Result { - let s = s.trim(); - if s.is_empty() { - return Err(miette::miette!("empty duration string")); - } - let (num_str, unit) = s.split_at(s.len() - 1); - let num: i64 = num_str - .parse() - .map_err(|_| miette::miette!("invalid duration: {s} (expected e.g. 5m, 1h, 30s)"))?; - let multiplier = match unit { - "s" => 1_000, - "m" => 60_000, - "h" => 3_600_000, - _ => { - return Err(miette::miette!( - "unknown duration unit: {unit} (use s, m, or h)" - )); - } - }; - Ok(num * multiplier) -} - -fn confirm_global_setting_takeover(key: &str, yes: bool) -> Result<()> { - if yes { - return Ok(()); - } - - if !std::io::stdin().is_terminal() || !std::io::stdout().is_terminal() { - return Err(miette::miette!( - "global setting updates require confirmation; pass --yes in non-interactive mode" - )); - } - - let proceed = Confirm::with_theme(&ColorfulTheme::default()) - .with_prompt(format!( - "Setting '{key}' globally will disable sandbox-level management for this key. Continue?" - )) - .default(false) - .interact() - .into_diagnostic()?; - - if !proceed { - return Err(miette::miette!("aborted by user")); - } - - Ok(()) -} - -fn confirm_global_setting_delete(key: &str, yes: bool) -> Result<()> { - if yes { - return Ok(()); - } - - if !std::io::stdin().is_terminal() || !std::io::stdout().is_terminal() { - return Err(miette::miette!( - "global setting deletes require confirmation; pass --yes in non-interactive mode" - )); - } - - let proceed = Confirm::with_theme(&ColorfulTheme::default()) - .with_prompt(format!( - "Deleting global setting '{key}' re-enables sandbox-level management for this key. Continue?" - )) - .default(false) - .interact() - .into_diagnostic()?; - - if !proceed { - return Err(miette::miette!("aborted by user")); - } - - Ok(()) -} - -fn parse_cli_setting_value(key: &str, raw_value: &str) -> Result { - let setting = settings::setting_for_key(key).ok_or_else(|| { - miette::miette!( - "unknown setting key '{}'. Allowed keys: {}", - key, - settings::registered_keys_csv() - ) - })?; - - let value = match setting.kind { - SettingValueKind::String => { - // Reject typos client-side so `openshell settings set ... - // proposal_approval_mode autom` errors immediately instead of - // round-tripping through the server. The server enforces the - // same check independently for non-CLI callers. - setting - .validate_string_value(raw_value) - .map_err(|allowed| { - miette::miette!( - "invalid value '{}' for key '{}'; expected one of: {}", - raw_value, - key, - allowed.join(", ") - ) - })?; - setting_value::Value::StringValue(raw_value.to_string()) - } - SettingValueKind::Int => { - let parsed = raw_value.trim().parse::().map_err(|_| { - miette::miette!( - "invalid int value '{}' for key '{}'; expected base-10 integer", - raw_value, - key - ) - })?; - setting_value::Value::IntValue(parsed) - } - SettingValueKind::Bool => { - let parsed = settings::parse_bool_like(raw_value).ok_or_else(|| { - miette::miette!( - "invalid bool value '{}' for key '{}'; expected one of: true,false,yes,no,1,0", - raw_value, - key - ) - })?; - setting_value::Value::BoolValue(parsed) - } - }; - - Ok(SettingValue { value: Some(value) }) -} - -fn format_setting_value(value: Option<&SettingValue>) -> String { - let Some(value) = value.and_then(|v| v.value.as_ref()) else { - return "".to_string(); - }; - match value { - setting_value::Value::StringValue(v) => v.clone(), - setting_value::Value::BoolValue(v) => v.to_string(), - setting_value::Value::IntValue(v) => v.to_string(), - setting_value::Value::BytesValue(v) => format!("", v.len()), - } -} - -fn short_hash(hash: &str) -> &str { - if hash.len() >= 12 { &hash[..12] } else { hash } -} - -fn print_policy_merge_warnings(warnings: &[openshell_policy::PolicyMergeWarning]) { - for warning in warnings { - eprintln!("{} {}", "!".yellow().bold(), warning); - } -} - pub async fn sandbox_policy_set_global( server: &str, policy_path: &str, @@ -8859,26 +7992,6 @@ fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String format!("{host_port} [{}]", tags.join(", ")) } -fn non_empty_or<'a>(value: &'a str, fallback: &'a str) -> &'a str { - if value.is_empty() { fallback } else { value } -} - -/// Format a millisecond timestamp into a readable string. -fn format_timestamp_ms(ms: i64) -> String { - if ms <= 0 { - return "-".to_string(); - } - let secs = ms / 1000; - let mins = (secs / 60) % 60; - let hours = (secs / 3600) % 24; - let days = secs / 86400; - if days > 0 { - format!("{days}d {hours:02}:{mins:02}") - } else { - format!("{hours:02}:{mins:02}") - } -} - #[cfg(test)] mod tests { use super::{ @@ -8892,13 +8005,14 @@ mod tests { inferred_provider_type, mtls_certs_exist_for_gateway, package_managed_tls_dirs, parse_cli_setting_value, parse_credential_expiry_cli_value, parse_credential_expiry_pairs, parse_credential_pairs, parse_driver_config_json, parse_secret_material_env_pairs, - plaintext_gateway_is_remote, policy_revision_to_json, progress_step_from_metadata, + plaintext_gateway_is_remote, policy_revision_to_json, provider_profile_allows_empty_credentials, provisioning_timeout_message, ready_false_condition_message, refresh_status_header, refresh_status_row, resolve_from, sandbox_should_persist, sandbox_upload_plan, service_expose_status_error, service_url_for_gateway, }; use crate::TEST_ENV_LOCK; + use crate::commands::common::progress_step_from_metadata; use hyper::StatusCode; use std::fs; use std::io::{Read, Write}; From f0b695a8b87ad415766438f0df83940fa6976627 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Mon, 20 Jul 2026 00:00:05 -0700 Subject: [PATCH 2/3] refactor(cli): narrow visibility and fix review issues in shared helpers extraction - Remove orphan doc comment left on sandbox_list after print_yaml_line extraction - Change `pub mod commands` to `pub(crate) mod commands` in lib.rs - Move 6 unnecessarily pub-exported items to private use imports in run.rs - Restore stripped rationale comment in parse_cli_setting_value Signed-off-by: Varsha Prasad Narsing --- crates/openshell-cli/src/commands/common.rs | 4 ++++ crates/openshell-cli/src/lib.rs | 2 +- crates/openshell-cli/src/run.rs | 17 +++++++---------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index b8fce0bc68..297b87c16c 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -620,6 +620,10 @@ pub fn parse_cli_setting_value(key: &str, raw_value: &str) -> Result { + // Reject typos client-side so `openshell settings set ... + // proposal_approval_mode autom` errors immediately instead of + // round-tripping through the server. The server enforces the + // same check independently for non-CLI callers. setting .validate_string_value(raw_value) .map_err(|allowed| { diff --git a/crates/openshell-cli/src/lib.rs b/crates/openshell-cli/src/lib.rs index bc5832e839..ecdf54f28f 100644 --- a/crates/openshell-cli/src/lib.rs +++ b/crates/openshell-cli/src/lib.rs @@ -9,7 +9,7 @@ pub(crate) static TEST_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); pub mod auth; -pub mod commands; +pub(crate) mod commands; pub mod completers; pub mod edge_tunnel; pub mod oidc_auth; diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index c155948c26..c6348f8942 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4,18 +4,17 @@ //! CLI command implementations. pub use crate::commands::common::{ - PolicyGetView, confirm_global_setting_delete, confirm_global_setting_takeover, - is_valid_env_name, parse_credential_expiry_cli_value, parse_credential_expiry_value, - parse_duration_to_ms, parse_env_pairs, parse_key_value_pairs, parse_secret_material_env_pairs, + PolicyGetView, parse_credential_expiry_cli_value, parse_env_pairs, parse_key_value_pairs, }; use crate::commands::common::{ - ProvisioningDisplay, ProvisioningStep, format_epoch_ms, format_optional_epoch_ms, + ProvisioningDisplay, ProvisioningStep, confirm_global_setting_delete, + confirm_global_setting_takeover, format_epoch_ms, format_optional_epoch_ms, format_setting_value, format_timestamp, format_timestamp_ms, handle_platform_progress_event, is_provisioning_progress_event, non_empty_or, parse_cli_setting_value, - parse_credential_expiry_pairs, parse_credential_pairs, phase_name, print_policy_merge_warnings, - print_sandbox_header, print_sandbox_policy, provisioning_timeout_message, - ready_false_condition_message, scrub_git_env, short_hash, truncate_display, - truncate_status_field, + parse_credential_expiry_pairs, parse_credential_pairs, parse_duration_to_ms, + parse_secret_material_env_pairs, phase_name, print_policy_merge_warnings, print_sandbox_header, + print_sandbox_policy, provisioning_timeout_message, ready_false_condition_message, + scrub_git_env, short_hash, truncate_display, truncate_status_field, }; use crate::policy_update::build_policy_update_plan; @@ -3024,8 +3023,6 @@ async fn sandbox_exec_interactive_grpc( Ok(exit_code) } -/// Print a single YAML line with dimmed keys and regular values. - /// List sandboxes. #[allow(clippy::too_many_arguments)] pub async fn sandbox_list( From b792fcc678ff98d7b885c9cee0cbaf110770c9ab Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Tue, 21 Jul 2026 23:56:32 -0700 Subject: [PATCH 3/3] style(cli): fix rustfmt import line wrapping in run.rs Signed-off-by: Varsha Prasad Narsing --- crates/openshell-cli/src/run.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index c6348f8942..e0cf59495d 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -58,10 +58,10 @@ use openshell_core::proto::{ ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileDiagnostic, ProviderProfileImportItem, RejectDraftChunkRequest, ResourceRequirements, RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy, - SandboxSpec, SandboxTemplate, ServiceEndpointResponse, ServiceStatus, - SetInferenceRouteRequest, SettingScope, TcpForwardFrame, TcpForwardInit, TcpRelayTarget, - UpdateConfigRequest, UpdateProviderProfilesRequest, UpdateProviderRequest, WatchSandboxRequest, - exec_sandbox_event, setting_value, tcp_forward_init, + SandboxSpec, SandboxTemplate, ServiceEndpointResponse, ServiceStatus, SetInferenceRouteRequest, + SettingScope, TcpForwardFrame, TcpForwardInit, TcpRelayTarget, UpdateConfigRequest, + UpdateProviderProfilesRequest, UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, + setting_value, tcp_forward_init, }; use openshell_core::settings; use openshell_core::{ObjectId, ObjectName, ObjectWorkspace};