From ef959bc0eadca999e4dfae160165532a4dad97d0 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Thu, 8 Oct 2026 10:29:42 +0100 Subject: [PATCH 1/2] feat(cli): add --annotation to sandbox create Signed-off-by: Eric Curtin --- crates/openshell-cli/src/commands/common.rs | 46 +++++++++++ crates/openshell-cli/src/main.rs | 50 +++++++++--- crates/openshell-cli/src/run.rs | 17 ++-- .../sandbox_create_lifecycle_integration.rs | 80 +++++++++++++++++++ docs/how-it-works/sandboxes/overview.mdx | 6 ++ skills/openshell-cli/SKILL.md | 1 + 6 files changed, 179 insertions(+), 21 deletions(-) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index 49fd66a9a8..4c87e1ac2a 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -891,6 +891,22 @@ pub fn parse_env_pairs(items: &[String]) -> Result> { Ok(map) } +/// Annotation keys under this prefix are managed by the system, not callers. +const RESERVED_ANNOTATION_PREFIX: &str = "openshell.nvidia.com/"; + +pub fn parse_annotation_pairs(items: &[String]) -> Result> { + let map = parse_key_value_pairs(items, "--annotation")?; + if let Some(key) = map + .keys() + .find(|key| key.starts_with(RESERVED_ANNOTATION_PREFIX)) + { + return Err(miette::miette!( + "--annotation keys starting with {RESERVED_ANNOTATION_PREFIX} 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> { @@ -1072,6 +1088,36 @@ pub fn scrub_git_env(command: &mut Command) -> &mut Command { mod tests { use super::*; + #[test] + fn parse_annotation_pairs_keeps_slashes_spaces_and_equals() { + let items = vec!["source=/home/you/my policy.yaml?a=b".to_string()]; + let map = parse_annotation_pairs(&items).expect("annotation should parse"); + assert_eq!( + map.get("source").map(String::as_str), + Some("/home/you/my policy.yaml?a=b") + ); + } + + #[test] + fn parse_annotation_pairs_rejects_malformed_and_empty_keys() { + let err = parse_annotation_pairs(&["nokey".to_string()]).expect_err("missing '='"); + assert!(err.to_string().contains("--annotation expects KEY=VALUE")); + + let err = parse_annotation_pairs(&["=value".to_string()]).expect_err("empty key"); + assert!(err.to_string().contains("--annotation key cannot be empty")); + } + + #[test] + fn parse_annotation_pairs_rejects_reserved_keys() { + let err = parse_annotation_pairs(&["openshell.nvidia.com/retention=ephemeral".to_string()]) + .expect_err("reserved key"); + assert!(err.to_string().contains("are reserved")); + + assert!( + parse_annotation_pairs(&["example.com/openshell.nvidia.com/x=1".to_string()]).is_ok() + ); + } + #[test] fn parse_duration_to_ms_parses_supported_units() { assert_eq!(parse_duration_to_ms("30s").expect("parse"), 30_000); diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index c0821ccc9f..3e9ea35f7f 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -8,7 +8,6 @@ use clap_complete::engine::ArgValueCompleter; use clap_complete::env::CompleteEnv; use miette::Result; use openshell_cli::color::{self, ColorChoice, Colorize}; -use std::collections::HashMap; use std::io::Write; use std::path::PathBuf; @@ -1580,6 +1579,11 @@ enum SandboxCommands { #[arg(long = "label")] labels: Vec, + /// Attach non-secret annotations to the sandbox (key=value format, + /// repeatable). Values may contain `/`, spaces, and `=`. + #[arg(long = "annotation", value_name = "KEY=VALUE")] + annotations: Vec, + /// Set a non-secret environment variable in the sandbox. /// Do not use this option for API keys, tokens, or other secrets; create /// a provider and attach it with `--provider` instead. Repeatable. @@ -3369,6 +3373,7 @@ async fn run_async() -> Result<()> { auto_providers, no_auto_providers, labels, + annotations, envs, no_credential_warnings, approval_mode, @@ -3393,18 +3398,9 @@ async fn run_async() -> Result<()> { None // prompt or auto-detect }; - // Parse --label flags into a HashMap. - let mut labels_map = HashMap::new(); - for label_str in &labels { - let parts: Vec<&str> = label_str.splitn(2, '=').collect(); - if parts.len() != 2 { - return Err(miette::miette!( - "invalid label format '{}', expected key=value", - label_str - )); - } - labels_map.insert(parts[0].to_string(), parts[1].to_string()); - } + // Parse --label and --annotation flags into maps. + let labels_map = run::parse_key_value_pairs(&labels, "--label")?; + let annotations_map = run::parse_annotation_pairs(&annotations)?; // Parse --env flags into a HashMap. let env_map = run::parse_env_pairs(&envs)?; @@ -3456,6 +3452,7 @@ async fn run_async() -> Result<()> { tty_override, auto_providers_override, labels: labels_map, + annotations: annotations_map, environment: env_map, approval_mode: &approval_mode, output: output.as_str(), @@ -6139,6 +6136,33 @@ mod tests { } } + #[test] + fn sandbox_create_annotation_is_repeatable() { + let cli = Cli::try_parse_from([ + "openshell", + "sandbox", + "create", + "--annotation", + "policy-source-file=/home/you/my policy.yaml", + "--annotation", + "owner=platform", + ]) + .unwrap(); + match cli.command { + Some(Commands::Sandbox { + command: Some(SandboxCommands::Create { annotations, .. }), + .. + }) => assert_eq!( + annotations, + vec![ + "policy-source-file=/home/you/my policy.yaml".to_string(), + "owner=platform".to_string(), + ] + ), + other => panic!("expected SandboxCommands::Create, got: {other:?}"), + } + } + #[test] fn sandbox_create_restart_policy_defaults_to_never() { let cli = Cli::try_parse_from(["openshell", "sandbox", "create"]).unwrap(); diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 830ebe3540..4631f940a6 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4,8 +4,8 @@ //! CLI command implementations. pub use crate::commands::common::{ - PolicyGetView, parse_credential_expiry_cli_value, parse_env_pairs, parse_key_value_pairs, - parse_secret_material_env_pairs, warn_credential_env_vars, + PolicyGetView, parse_annotation_pairs, parse_credential_expiry_cli_value, parse_env_pairs, + parse_key_value_pairs, parse_secret_material_env_pairs, warn_credential_env_vars, }; use crate::commands::common::{ ProvisioningDisplay, ProvisioningStep, confirm_global_setting_delete, @@ -450,6 +450,7 @@ pub struct SandboxCreateConfig<'a> { pub tty_override: Option, pub auto_providers_override: Option, pub labels: HashMap, + pub annotations: HashMap, pub environment: HashMap, pub approval_mode: &'a str, pub output: &'a str, @@ -480,6 +481,7 @@ impl Default for SandboxCreateConfig<'_> { tty_override: None, auto_providers_override: None, labels: HashMap::new(), + annotations: HashMap::new(), environment: HashMap::new(), approval_mode: "manual", output: "table", @@ -518,6 +520,7 @@ pub async fn sandbox_create( tty_override, auto_providers_override, labels, + mut annotations, environment, approval_mode, output, @@ -662,14 +665,12 @@ pub async fn sandbox_create( && command.is_empty() && (!std::io::stdin().is_terminal() || !std::io::stdout().is_terminal())); let await_main_process_attachment = output == "table" && editor.is_none() && !create_detaches; - let annotations = if persist { - HashMap::new() - } else { - HashMap::from([( + if !persist { + annotations.insert( "openshell.nvidia.com/retention".to_string(), "ephemeral".to_string(), - )]) - }; + ); + } let request = CreateSandboxRequest { request_id: String::new(), spec: Some(SandboxSpec { diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index bb0abceb2f..d3f7477625 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -2000,6 +2000,86 @@ async fn detached_ephemeral_command_delegates_cleanup_to_gateway() { ); } +#[tokio::test] +async fn sandbox_create_sends_annotations() { + let server = run_server().await; + let fake_ssh_dir = tempfile::tempdir().unwrap(); + let xdg_dir = tempfile::tempdir().unwrap(); + let _env = test_env(&fake_ssh_dir, &xdg_dir); + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + let source = "/home/you/my policy.yaml=1".to_string(); + run::sandbox_create( + &server.endpoint, + "openshell", + run::SandboxCreateConfig { + name: Some("annotated"), + keep: true, + command: &["echo".into(), "OK".into()], + annotations: HashMap::from([("policy-source-file".to_string(), source.clone())]), + ..test_config() + }, + "default", + &tls, + ) + .await + .expect("sandbox create should succeed"); + + let requests = create_requests(&server).await; + assert_eq!( + requests[0] + .annotations + .get("policy-source-file") + .map(String::as_str), + Some(source.as_str()) + ); + assert!( + !requests[0] + .annotations + .contains_key("openshell.nvidia.com/retention"), + "a kept sandbox must not be marked ephemeral" + ); +} + +#[tokio::test] +async fn ephemeral_sandbox_create_keeps_annotations_and_retention() { + let server = run_server().await; + let fake_ssh_dir = tempfile::tempdir().unwrap(); + let xdg_dir = tempfile::tempdir().unwrap(); + let _env = test_env(&fake_ssh_dir, &xdg_dir); + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + run::sandbox_create( + &server.endpoint, + "openshell", + run::SandboxCreateConfig { + name: Some("annotated-ephemeral"), + keep: false, + command: &["echo".into(), "OK".into()], + annotations: HashMap::from([("owner".to_string(), "platform".to_string())]), + ..test_config() + }, + "default", + &tls, + ) + .await + .expect("sandbox create should succeed"); + + let annotations = &create_requests(&server).await[0].annotations; + assert_eq!( + annotations.get("owner").map(String::as_str), + Some("platform") + ); + assert_eq!( + annotations + .get("openshell.nvidia.com/retention") + .map(String::as_str), + Some("ephemeral") + ); +} + #[tokio::test] async fn sandbox_create_sends_driver_config_json() { let server = run_server().await; diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index fcd71bdb92..7076e08033 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -414,6 +414,12 @@ openshell sandbox list --selector env=dev openshell sandbox list --selector env=dev,team=platform ``` +Use `--annotation KEY=VALUE` for non-secret metadata that does not fit label limits, such as a file path. Annotation values can contain `/`, spaces, and `=`, and appear in the `annotations` field of `sandbox get -o json` and `sandbox list -o json`. Keys starting with `openshell.nvidia.com/` are reserved. + +```shell +openshell sandbox create --annotation "policy-source-file=/home/you/policies/research policy.yaml" -- claude +``` + The Python SDK accepts the same gateway labels and selectors. Labels passed to `create` are stored on the gateway sandbox object and are returned on `SandboxRef.labels`, so selector-based listing finds Python-created sandboxes: diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index f99b6cb7ed..d9cb7ec436 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -275,6 +275,7 @@ Key flags: - `--driver-config-json`: Pass experimental driver-specific sandbox configuration - `--template NAME`: Create from a named sandbox workload template. Conflicts with inline workload flags such as `--from`, `--gpu`, `--cpu`, `--memory`, `--env`, and `--driver-config-json`. - `--label KEY=VALUE`: Add labels for later selection (repeatable) +- `--annotation KEY=VALUE`: Add non-secret metadata such as file paths; keys starting with `openshell.nvidia.com/` are reserved (repeatable) - `--env KEY=VALUE`: Set non-secret sandbox environment variables (repeatable); use `--provider` for credentials - `--tty`: Allocate a retained PTY for the canonical main process - `--restart-policy never|on-failure|always`: Select gateway-owned main-process restart behavior; `never` is the default From 2c1004639c36d1178093ae057542de0eabb58f12 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Thu, 8 Oct 2026 22:04:51 +0100 Subject: [PATCH 2/2] fix(cli): drop reserved annotation check Signed-off-by: Eric Curtin --- crates/openshell-cli/src/commands/common.rs | 39 ++++----------------- crates/openshell-cli/src/main.rs | 2 +- crates/openshell-cli/src/run.rs | 4 +-- docs/how-it-works/sandboxes/overview.mdx | 2 +- skills/openshell-cli/SKILL.md | 2 +- 5 files changed, 12 insertions(+), 37 deletions(-) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index 4c87e1ac2a..a6d2b7d37f 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -891,22 +891,6 @@ pub fn parse_env_pairs(items: &[String]) -> Result> { Ok(map) } -/// Annotation keys under this prefix are managed by the system, not callers. -const RESERVED_ANNOTATION_PREFIX: &str = "openshell.nvidia.com/"; - -pub fn parse_annotation_pairs(items: &[String]) -> Result> { - let map = parse_key_value_pairs(items, "--annotation")?; - if let Some(key) = map - .keys() - .find(|key| key.starts_with(RESERVED_ANNOTATION_PREFIX)) - { - return Err(miette::miette!( - "--annotation keys starting with {RESERVED_ANNOTATION_PREFIX} 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> { @@ -1089,9 +1073,9 @@ mod tests { use super::*; #[test] - fn parse_annotation_pairs_keeps_slashes_spaces_and_equals() { + fn parse_key_value_pairs_keeps_slashes_spaces_and_equals() { let items = vec!["source=/home/you/my policy.yaml?a=b".to_string()]; - let map = parse_annotation_pairs(&items).expect("annotation should parse"); + let map = parse_key_value_pairs(&items, "--annotation").expect("annotation should parse"); assert_eq!( map.get("source").map(String::as_str), Some("/home/you/my policy.yaml?a=b") @@ -1099,25 +1083,16 @@ mod tests { } #[test] - fn parse_annotation_pairs_rejects_malformed_and_empty_keys() { - let err = parse_annotation_pairs(&["nokey".to_string()]).expect_err("missing '='"); + fn parse_key_value_pairs_rejects_malformed_and_empty_keys() { + let err = + parse_key_value_pairs(&["nokey".to_string()], "--annotation").expect_err("missing '='"); assert!(err.to_string().contains("--annotation expects KEY=VALUE")); - let err = parse_annotation_pairs(&["=value".to_string()]).expect_err("empty key"); + let err = + parse_key_value_pairs(&["=value".to_string()], "--annotation").expect_err("empty key"); assert!(err.to_string().contains("--annotation key cannot be empty")); } - #[test] - fn parse_annotation_pairs_rejects_reserved_keys() { - let err = parse_annotation_pairs(&["openshell.nvidia.com/retention=ephemeral".to_string()]) - .expect_err("reserved key"); - assert!(err.to_string().contains("are reserved")); - - assert!( - parse_annotation_pairs(&["example.com/openshell.nvidia.com/x=1".to_string()]).is_ok() - ); - } - #[test] fn parse_duration_to_ms_parses_supported_units() { assert_eq!(parse_duration_to_ms("30s").expect("parse"), 30_000); diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 3e9ea35f7f..f62aaf6eb2 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -3400,7 +3400,7 @@ async fn run_async() -> Result<()> { // Parse --label and --annotation flags into maps. let labels_map = run::parse_key_value_pairs(&labels, "--label")?; - let annotations_map = run::parse_annotation_pairs(&annotations)?; + let annotations_map = run::parse_key_value_pairs(&annotations, "--annotation")?; // Parse --env flags into a HashMap. let env_map = run::parse_env_pairs(&envs)?; diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 4631f940a6..2c8ccebc6c 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4,8 +4,8 @@ //! CLI command implementations. pub use crate::commands::common::{ - PolicyGetView, parse_annotation_pairs, parse_credential_expiry_cli_value, parse_env_pairs, - parse_key_value_pairs, parse_secret_material_env_pairs, warn_credential_env_vars, + PolicyGetView, parse_credential_expiry_cli_value, parse_env_pairs, parse_key_value_pairs, + parse_secret_material_env_pairs, warn_credential_env_vars, }; use crate::commands::common::{ ProvisioningDisplay, ProvisioningStep, confirm_global_setting_delete, diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index 7076e08033..8ad15993eb 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -414,7 +414,7 @@ openshell sandbox list --selector env=dev openshell sandbox list --selector env=dev,team=platform ``` -Use `--annotation KEY=VALUE` for non-secret metadata that does not fit label limits, such as a file path. Annotation values can contain `/`, spaces, and `=`, and appear in the `annotations` field of `sandbox get -o json` and `sandbox list -o json`. Keys starting with `openshell.nvidia.com/` are reserved. +Use `--annotation KEY=VALUE` for non-secret metadata that does not fit label limits, such as a file path. Annotation values can contain `/`, spaces, and `=`, and appear in the `annotations` field of `sandbox get -o json` and `sandbox list -o json`. ```shell openshell sandbox create --annotation "policy-source-file=/home/you/policies/research policy.yaml" -- claude diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index d9cb7ec436..4430e68594 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -275,7 +275,7 @@ Key flags: - `--driver-config-json`: Pass experimental driver-specific sandbox configuration - `--template NAME`: Create from a named sandbox workload template. Conflicts with inline workload flags such as `--from`, `--gpu`, `--cpu`, `--memory`, `--env`, and `--driver-config-json`. - `--label KEY=VALUE`: Add labels for later selection (repeatable) -- `--annotation KEY=VALUE`: Add non-secret metadata such as file paths; keys starting with `openshell.nvidia.com/` are reserved (repeatable) +- `--annotation KEY=VALUE`: Add non-secret metadata such as file paths (repeatable) - `--env KEY=VALUE`: Set non-secret sandbox environment variables (repeatable); use `--provider` for credentials - `--tty`: Allocate a retained PTY for the canonical main process - `--restart-policy never|on-failure|always`: Select gateway-owned main-process restart behavior; `never` is the default