From 2f1008e15fc2c46f656853252319024c1d6bbf99 Mon Sep 17 00:00:00 2001 From: Artem Lytvyn Date: Fri, 7 Aug 2026 16:29:18 +0100 Subject: [PATCH 1/3] feat(cli): add credential env match validation Signed-off-by: Artem Lytvyn --- crates/openshell-cli/src/commands/common.rs | 132 ++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index e6edb4d33a..a89700c72f 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -16,6 +16,7 @@ use openshell_core::proto::{ PlatformEvent, SandboxPhase, SandboxPolicy, SettingValue, setting_value, }; use openshell_core::settings::{self, SettingValueKind}; +use openshell_providers::builtin_profiles; use owo_colors::OwoColorize; use std::collections::HashMap; use std::io::IsTerminal; @@ -743,6 +744,56 @@ pub fn parse_duration_to_ms(s: &str) -> Result { // Parsing utilities // --------------------------------------------------------------------------- +#[derive(Debug, Clone, PartialEq)] +pub struct ProfileSuggestion { + pub provider_type: String, + pub credential: String, +} + +fn credential_env_matches(env: &HashMap) -> Vec<(String, Vec)> { + const SUFFIXES: [&str; 7_usize] = [ + "_TOKEN", + "_SECRET", + "_PASSWORD", + "_CREDENTIAL", + "_ACCESS_KEY", + "_SECRET_KEY", + "_API_KEY", + ]; + let looks_like_credential = |key: &str| -> bool { + let upper = key.to_ascii_uppercase(); + SUFFIXES.iter().any(|s| upper.ends_with(*s)) + }; + + // scan builtin_profiles() + fn profile_suggestions(key: &str) -> Vec { + let mut suggestions = Vec::new(); + for profile in builtin_profiles() { + for cred in &profile.credentials { + if cred.env_vars.iter().any(|v| v.eq_ignore_ascii_case(key)) { + suggestions.push(ProfileSuggestion { + provider_type: profile.id.clone(), + credential: cred.name.clone(), + }); + } + } + } + suggestions + } + + let mut matches = Vec::new(); + + for key in env.keys() { + let sug = profile_suggestions(key); + if !sug.is_empty() || looks_like_credential(key) { + matches.push((key.clone(), sug)); + } + } + + matches.sort_by(|a, b| a.0.cmp(&b.0)); + matches +} + pub fn parse_key_value_pairs(items: &[String], flag: &str) -> Result> { let mut map = HashMap::new(); @@ -975,4 +1026,85 @@ mod tests { let err = parse_duration_to_ms("\u{20ac}").expect_err("missing number should error"); assert!(err.to_string().contains("invalid duration")); } + + // helper for building input + fn env(pairs: &[(&str, &str)]) -> HashMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + + #[test] + fn suffix_matche_no_profile() { + let env = env(&[("FOO_TOKEN", "x")]); + let prof = credential_env_matches(&env); + assert_eq!(prof.len(), 1_usize); + assert_eq!(&prof[0].0, "FOO_TOKEN"); + assert!(prof[0].1.is_empty()); + } + + #[test] + fn exact_profile_match() { + let env = env(&[("GITHUB_TOKEN", "x")]); + + let prof = credential_env_matches(&env); + assert_eq!(prof.len(), 1_usize); + assert_eq!(prof[0].0, "GITHUB_TOKEN"); + + let sug = &prof[0].1; + assert_eq!(sug.len(), 2_usize); + + assert_eq!(sug[0].provider_type, "copilot"); + assert_eq!(sug[0].credential, "api_token"); + + assert_eq!(sug[1].provider_type, "github"); + assert_eq!(sug[1].credential, "api_token"); + } + + #[test] + fn case_insensitive() { + let env = env(&[("gh_token", "x")]); + + let prof = credential_env_matches(&env); + assert_eq!(prof.len(), 1_usize); + assert_eq!(prof[0].0, "gh_token"); + + let sug = &prof[0].1; + assert_eq!(sug.len(), 2_usize); + + assert_eq!(sug[0].provider_type, "copilot"); + assert_eq!(sug[0].credential, "api_token"); + + assert_eq!(sug[1].provider_type, "github"); + assert_eq!(sug[1].credential, "api_token"); + } + + #[test] + fn non_credential_skipped() { + let env = env(&[("PATH", "x"), ("HOME", "y")]); + + let prof = credential_env_matches(&env); + assert!(prof.is_empty()); + } + + #[test] + fn no_value_leak() { + let env = env(&[("APP_SECRET", "secret")]); + + let prof = credential_env_matches(&env); + assert_eq!(prof.len(), 1_usize); + + let dumped = format!("{:?}", prof); + assert!(!dumped.contains("secret"), "value leaked: {}", dumped); + } + + #[test] + fn deterministic_order() { + let env = env(&[("ZED_TOKEN", "a"), ("ABC_SECRET", "b"), ("MID_PASSWORD", "c")]); + + let prof = credential_env_matches(&env); + let keys: Vec<&str> = prof.iter().map(|(k, _)| k.as_str()).collect(); + assert_eq!(keys, ["ABC_SECRET", "MID_PASSWORD", "ZED_TOKEN"]); + } } From 1b19bb1862634cff4727e6c05bef0fe38f550b37 Mon Sep 17 00:00:00 2001 From: Artem Lytvyn Date: Fri, 7 Aug 2026 16:56:38 +0100 Subject: [PATCH 2/3] feat(cli): warn when --env values look like credentials Signed-off-by: Artem Lytvyn --- crates/openshell-cli/src/commands/common.rs | 56 ++++++++++++++++++--- crates/openshell-cli/src/main.rs | 6 +++ crates/openshell-cli/src/run.rs | 2 +- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index a89700c72f..1a6cc23860 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -23,6 +23,8 @@ use std::io::IsTerminal; use std::process::Command; use std::time::{Duration, Instant}; +const DOCS_PROVIDERS_URL: &str = "https://docs.nvidia.com/openshell/latest/sandboxes/providers-v2"; + // --------------------------------------------------------------------------- // View types // --------------------------------------------------------------------------- @@ -744,7 +746,7 @@ pub fn parse_duration_to_ms(s: &str) -> Result { // Parsing utilities // --------------------------------------------------------------------------- -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct ProfileSuggestion { pub provider_type: String, pub credential: String, @@ -766,7 +768,7 @@ fn credential_env_matches(env: &HashMap) -> Vec<(String, Vec Vec { + let profile_suggestions = |key: &str| -> Vec { let mut suggestions = Vec::new(); for profile in builtin_profiles() { for cred in &profile.credentials { @@ -779,7 +781,7 @@ fn credential_env_matches(env: &HashMap) -> Vec<(String, Vec) -> Vec<(String, Vec, suppress: bool) { + if suppress { + return; + } + + let matches = credential_env_matches(env); + if matches.is_empty() { + return; + } + + for (key, suggestions) in &matches { + eprintln!( + "{} {key} looks like a credential passed as a plain environment variable.", + "⚠".yellow() + ); + eprintln!(" The agent inside the sandbox can read this value directly."); + eprintln!(); + + if suggestions.is_empty() { + eprintln!(" To hide it from the agent, use a provider instead of --env."); + } else { + eprintln!(" To hide it from the agent, use a provider instead:"); + for s in suggestions { + eprintln!( + " openshell provider create --name my-{ty} --type {ty} --credential {key}", + ty = s.provider_type + ); + } + eprintln!(" openshell sandbox create --provider my- ..."); + } + eprintln!(" See: {DOCS_PROVIDERS_URL}"); + eprintln!(); + } +} + pub fn parse_key_value_pairs(items: &[String], flag: &str) -> Result> { let mut map = HashMap::new(); @@ -1091,17 +1129,21 @@ mod tests { #[test] fn no_value_leak() { let env = env(&[("APP_SECRET", "secret")]); - + let prof = credential_env_matches(&env); assert_eq!(prof.len(), 1_usize); - let dumped = format!("{:?}", prof); - assert!(!dumped.contains("secret"), "value leaked: {}", dumped); + let dumped = format!("{prof:?}"); + assert!(!dumped.contains("secret"), "value leaked: {dumped}"); } #[test] fn deterministic_order() { - let env = env(&[("ZED_TOKEN", "a"), ("ABC_SECRET", "b"), ("MID_PASSWORD", "c")]); + let env = env(&[ + ("ZED_TOKEN", "a"), + ("ABC_SECRET", "b"), + ("MID_PASSWORD", "c"), + ]); let prof = credential_env_matches(&env); let keys: Vec<&str> = prof.iter().map(|(k, _)| k.as_str()).collect(); diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 4ea2765d25..d7be419f92 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1439,6 +1439,10 @@ enum SandboxCommands { #[arg(long = "env", value_name = "KEY=VALUE")] envs: Vec, + /// Suppress warnings when --env values look like credentials. + #[arg(long = "no-credential-warnings")] + no_credential_warnings: bool, + /// Approval mode for agent-authored policy proposals. /// /// `manual` (default): every proposal lands in the draft inbox for @@ -2934,6 +2938,7 @@ async fn main() -> Result<()> { no_auto_providers, labels, envs, + no_credential_warnings, approval_mode, output, command, @@ -2971,6 +2976,7 @@ async fn main() -> Result<()> { // Parse --env flags into a HashMap. let env_map = run::parse_env_pairs(&envs)?; + run::warn_credential_env_vars(&env_map, no_credential_warnings); // Parse --upload specs into [(local_path, sandbox_path, git_ignore)]. let upload_specs: Vec<(String, Option, bool)> = upload diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 48bf2d3dd5..51f365878b 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5,7 +5,7 @@ pub use crate::commands::common::{ PolicyGetView, parse_credential_expiry_cli_value, parse_env_pairs, parse_key_value_pairs, - parse_secret_material_env_pairs, + parse_secret_material_env_pairs, warn_credential_env_vars, }; use crate::commands::common::{ ProvisioningDisplay, ProvisioningStep, confirm_global_setting_delete, From e594d04e077b7b2f32e4b2544f93c3472fcbe802 Mon Sep 17 00:00:00 2001 From: Artem Lytvyn Date: Fri, 7 Aug 2026 17:02:28 +0100 Subject: [PATCH 3/3] docs(sandbox): add flag --no-credential-warnings details + polishing Signed-off-by: Artem Lytvyn --- crates/openshell-cli/src/commands/common.rs | 22 ++++++++++----------- docs/sandboxes/manage-sandboxes.mdx | 2 ++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index 1a6cc23860..5098bd11ad 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -754,17 +754,17 @@ pub struct ProfileSuggestion { fn credential_env_matches(env: &HashMap) -> Vec<(String, Vec)> { const SUFFIXES: [&str; 7_usize] = [ - "_TOKEN", - "_SECRET", - "_PASSWORD", - "_CREDENTIAL", - "_ACCESS_KEY", - "_SECRET_KEY", - "_API_KEY", + "TOKEN", + "SECRET", + "PASSWORD", + "CREDENTIAL", + "ACCESS_KEY", + "SECRET_KEY", + "API_KEY", ]; let looks_like_credential = |key: &str| -> bool { let upper = key.to_ascii_uppercase(); - SUFFIXES.iter().any(|s| upper.ends_with(*s)) + SUFFIXES.iter().any(|s| upper.contains(*s)) }; // scan builtin_profiles() @@ -1074,7 +1074,7 @@ mod tests { } #[test] - fn suffix_matche_no_profile() { + fn suffix_match_no_profile() { let env = env(&[("FOO_TOKEN", "x")]); let prof = credential_env_matches(&env); assert_eq!(prof.len(), 1_usize); @@ -1128,13 +1128,13 @@ mod tests { #[test] fn no_value_leak() { - let env = env(&[("APP_SECRET", "secret")]); + let env = env(&[("APP_SECRET", "secretVALUE42")]); let prof = credential_env_matches(&env); assert_eq!(prof.len(), 1_usize); let dumped = format!("{prof:?}"); - assert!(!dumped.contains("secret"), "value leaked: {dumped}"); + assert!(!dumped.contains("secretVALUE42"), "value leaked: {dumped}"); } #[test] diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index bc408c4ecd..5d4fad33af 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -202,6 +202,8 @@ openshell sandbox create --env API_KEY=sk-test --env DEBUG=1 -- my-agent Variables set with `--env` are available to all processes in the sandbox, including interactive shells and exec commands. +When an `--env` key looks like a credential — a known provider variable or a name ending in `_TOKEN`, `_SECRET`, `_API_KEY`, and similar — `sandbox create` prints a non-blocking warning. The agent inside the sandbox can read plain environment values directly, so to hide a secret from the agent, attach it through a [provider](/sandboxes/providers-v2) with `--provider` instead. Suppress the warning with `--no-credential-warnings`. Detection uses the key name only; values are never inspected or printed. + You can also set per-command environment variables with `sandbox exec`: ```shell