Skip to content

Commit dd2b4e3

Browse files
authored
feat(cli): warn when --env values look like credentials (#2655)
* feat(cli): add credential env match validation Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * feat(cli): warn when --env values look like credentials Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * docs(sandbox): add flag --no-credential-warnings details + polishing Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(cli): match credential keywords on underscore segments Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> --------- Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
1 parent 2f96c53 commit dd2b4e3

4 files changed

Lines changed: 216 additions & 1 deletion

File tree

crates/openshell-cli/src/commands/common.rs

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@ use openshell_core::proto::{
1616
PlatformEvent, SandboxPhase, SandboxPolicy, SettingValue, setting_value,
1717
};
1818
use openshell_core::settings::{self, SettingValueKind};
19+
use openshell_providers::builtin_profiles;
1920
use owo_colors::OwoColorize;
2021
use std::collections::HashMap;
2122
use std::io::IsTerminal;
2223
use std::process::Command;
2324
use std::time::{Duration, Instant};
2425

26+
const DOCS_PROVIDERS_URL: &str = "https://docs.nvidia.com/openshell/latest/sandboxes/providers-v2";
27+
2528
// ---------------------------------------------------------------------------
2629
// View types
2730
// ---------------------------------------------------------------------------
@@ -743,6 +746,96 @@ pub fn parse_duration_to_ms(s: &str) -> Result<i64> {
743746
// Parsing utilities
744747
// ---------------------------------------------------------------------------
745748

749+
#[derive(Debug, Clone, PartialEq, Eq)]
750+
pub struct ProfileSuggestion {
751+
pub provider_type: String,
752+
pub credential: String,
753+
}
754+
755+
fn credential_env_matches(env: &HashMap<String, String>) -> Vec<(String, Vec<ProfileSuggestion>)> {
756+
const KEYWORDS: [&str; 7_usize] = [
757+
"TOKEN",
758+
"SECRET",
759+
"PASSWORD",
760+
"CREDENTIAL",
761+
"ACCESS_KEY",
762+
"SECRET_KEY",
763+
"API_KEY",
764+
];
765+
let looks_like_credential = |key: &str| -> bool {
766+
let upper = key.to_ascii_uppercase();
767+
let segs = upper.split('_').collect::<Vec<&str>>();
768+
KEYWORDS.iter().any(|kw| {
769+
let words = kw.split('_').collect::<Vec<&str>>();
770+
segs.windows(words.len()).any(|w| w == words.as_slice())
771+
})
772+
};
773+
774+
// scan builtin_profiles()
775+
let profile_suggestions = |key: &str| -> Vec<ProfileSuggestion> {
776+
let mut suggestions = Vec::new();
777+
for profile in builtin_profiles() {
778+
for cred in &profile.credentials {
779+
if cred.env_vars.iter().any(|v| v.eq_ignore_ascii_case(key)) {
780+
suggestions.push(ProfileSuggestion {
781+
provider_type: profile.id.clone(),
782+
credential: cred.name.clone(),
783+
});
784+
}
785+
}
786+
}
787+
suggestions
788+
};
789+
790+
let mut matches = Vec::new();
791+
792+
for key in env.keys() {
793+
let sug = profile_suggestions(key);
794+
if !sug.is_empty() || looks_like_credential(key) {
795+
matches.push((key.clone(), sug));
796+
}
797+
}
798+
799+
matches.sort_by(|a, b| a.0.cmp(&b.0));
800+
matches
801+
}
802+
803+
#[allow(clippy::implicit_hasher)]
804+
pub fn warn_credential_env_vars(env: &HashMap<String, String>, suppress: bool) {
805+
if suppress {
806+
return;
807+
}
808+
809+
let matches = credential_env_matches(env);
810+
if matches.is_empty() {
811+
return;
812+
}
813+
814+
for (key, suggestions) in &matches {
815+
eprintln!(
816+
"{} {key} looks like a credential passed as a plain environment variable.",
817+
"⚠".yellow()
818+
);
819+
eprintln!(" The agent inside the sandbox can read this value directly.");
820+
eprintln!();
821+
822+
if suggestions.is_empty() {
823+
eprintln!(" To hide it from the agent, use a provider instead of --env.");
824+
} else {
825+
eprintln!(" To hide it from the agent, use a provider instead:");
826+
for s in suggestions {
827+
eprintln!(
828+
" openshell provider create --name my-{ty} --type {ty} --credential {key}",
829+
ty = s.provider_type
830+
);
831+
}
832+
eprintln!(" openshell sandbox create --provider my-<name> ...");
833+
}
834+
eprintln!(" See: {DOCS_PROVIDERS_URL}");
835+
eprintln!();
836+
}
837+
}
838+
746839
pub fn parse_key_value_pairs(items: &[String], flag: &str) -> Result<HashMap<String, String>> {
747840
let mut map = HashMap::new();
748841

@@ -975,4 +1068,118 @@ mod tests {
9751068
let err = parse_duration_to_ms("\u{20ac}").expect_err("missing number should error");
9761069
assert!(err.to_string().contains("invalid duration"));
9771070
}
1071+
1072+
// helper for building input
1073+
fn env(pairs: &[(&str, &str)]) -> HashMap<String, String> {
1074+
pairs
1075+
.iter()
1076+
.map(|(k, v)| (k.to_string(), v.to_string()))
1077+
.collect()
1078+
}
1079+
1080+
#[test]
1081+
fn suffix_match_no_profile() {
1082+
let env = env(&[("FOO_TOKEN", "x")]);
1083+
let prof = credential_env_matches(&env);
1084+
assert_eq!(prof.len(), 1_usize);
1085+
assert_eq!(&prof[0].0, "FOO_TOKEN");
1086+
assert!(prof[0].1.is_empty());
1087+
}
1088+
1089+
#[test]
1090+
fn exact_profile_match() {
1091+
let env = env(&[("GITHUB_TOKEN", "x")]);
1092+
1093+
let prof = credential_env_matches(&env);
1094+
assert_eq!(prof.len(), 1_usize);
1095+
assert_eq!(prof[0].0, "GITHUB_TOKEN");
1096+
1097+
let sug = &prof[0].1;
1098+
assert_eq!(sug.len(), 2_usize);
1099+
1100+
assert_eq!(sug[0].provider_type, "copilot");
1101+
assert_eq!(sug[0].credential, "api_token");
1102+
1103+
assert_eq!(sug[1].provider_type, "github");
1104+
assert_eq!(sug[1].credential, "api_token");
1105+
}
1106+
1107+
#[test]
1108+
fn case_insensitive() {
1109+
let env = env(&[("gh_token", "x")]);
1110+
1111+
let prof = credential_env_matches(&env);
1112+
assert_eq!(prof.len(), 1_usize);
1113+
assert_eq!(prof[0].0, "gh_token");
1114+
1115+
let sug = &prof[0].1;
1116+
assert_eq!(sug.len(), 2_usize);
1117+
1118+
assert_eq!(sug[0].provider_type, "copilot");
1119+
assert_eq!(sug[0].credential, "api_token");
1120+
1121+
assert_eq!(sug[1].provider_type, "github");
1122+
assert_eq!(sug[1].credential, "api_token");
1123+
}
1124+
1125+
#[test]
1126+
fn non_credential_skipped() {
1127+
let env = env(&[("PATH", "x"), ("HOME", "y")]);
1128+
1129+
let prof = credential_env_matches(&env);
1130+
assert!(prof.is_empty());
1131+
}
1132+
1133+
#[test]
1134+
fn no_value_leak() {
1135+
let env = env(&[("APP_SECRET", "secretVALUE42")]);
1136+
1137+
let prof = credential_env_matches(&env);
1138+
assert_eq!(prof.len(), 1_usize);
1139+
1140+
let dumped = format!("{prof:?}");
1141+
assert!(!dumped.contains("secretVALUE42"), "value leaked: {dumped}");
1142+
}
1143+
1144+
#[test]
1145+
fn deterministic_order() {
1146+
let env = env(&[
1147+
("ZED_TOKEN", "a"),
1148+
("ABC_SECRET", "b"),
1149+
("MID_PASSWORD", "c"),
1150+
]);
1151+
1152+
let prof = credential_env_matches(&env);
1153+
let keys: Vec<&str> = prof.iter().map(|(k, _)| k.as_str()).collect();
1154+
assert_eq!(keys, ["ABC_SECRET", "MID_PASSWORD", "ZED_TOKEN"]);
1155+
}
1156+
1157+
#[test]
1158+
fn nonsecrets() {
1159+
let env = env(&[
1160+
("TOKENIZERS_PARALLELISM", "x"),
1161+
("PASSWORDLESS_LOGIN", "y"),
1162+
("SECRETARY_EMAIL", "z"),
1163+
]);
1164+
1165+
let prof = credential_env_matches(&env);
1166+
assert!(prof.is_empty());
1167+
}
1168+
1169+
#[test]
1170+
fn segment_matches() {
1171+
let env = env(&[
1172+
("DB_TOKEN", "a"),
1173+
("MY_ACCESS_KEY", "b"),
1174+
("PRIMARY_KEY", "c"),
1175+
]);
1176+
1177+
let prof = credential_env_matches(&env);
1178+
assert_eq!(prof.len(), 2_usize);
1179+
1180+
let keys = prof.iter().map(|(k, _)| k.as_str()).collect::<Vec<&str>>();
1181+
assert!(keys.contains(&"DB_TOKEN"));
1182+
assert!(keys.contains(&"MY_ACCESS_KEY"));
1183+
assert!(!keys.contains(&"PRIMARY_KEY"));
1184+
}
9781185
}

crates/openshell-cli/src/main.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,10 @@ enum SandboxCommands {
14391439
#[arg(long = "env", value_name = "KEY=VALUE")]
14401440
envs: Vec<String>,
14411441

1442+
/// Suppress warnings when --env values look like credentials.
1443+
#[arg(long = "no-credential-warnings")]
1444+
no_credential_warnings: bool,
1445+
14421446
/// Approval mode for agent-authored policy proposals.
14431447
///
14441448
/// `manual` (default): every proposal lands in the draft inbox for
@@ -2959,6 +2963,7 @@ async fn run_async() -> Result<()> {
29592963
no_auto_providers,
29602964
labels,
29612965
envs,
2966+
no_credential_warnings,
29622967
approval_mode,
29632968
output,
29642969
command,
@@ -2996,6 +3001,7 @@ async fn run_async() -> Result<()> {
29963001

29973002
// Parse --env flags into a HashMap<String, String>.
29983003
let env_map = run::parse_env_pairs(&envs)?;
3004+
run::warn_credential_env_vars(&env_map, no_credential_warnings);
29993005

30003006
// Parse --upload specs into [(local_path, sandbox_path, git_ignore)].
30013007
let upload_specs: Vec<(String, Option<String>, bool)> = upload

crates/openshell-cli/src/run.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
pub use crate::commands::common::{
77
PolicyGetView, parse_credential_expiry_cli_value, parse_env_pairs, parse_key_value_pairs,
8-
parse_secret_material_env_pairs,
8+
parse_secret_material_env_pairs, warn_credential_env_vars,
99
};
1010
use crate::commands::common::{
1111
ProvisioningDisplay, ProvisioningStep, confirm_global_setting_delete,

docs/sandboxes/manage-sandboxes.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ openshell sandbox create --env API_KEY=sk-test --env DEBUG=1 -- my-agent
202202
203203
Variables set with `--env` are available to all processes in the sandbox, including interactive shells and exec commands.
204204
205+
When an `--env` key looks like a credential — a known provider variable, or a name whose underscore-separated segments include a credential word such as `TOKEN`, `SECRET`, `PASSWORD`, `CREDENTIAL`, `API_KEY`, `ACCESS_KEY`, or `SECRET_KEY` (for example `DB_TOKEN` or `MY_ACCESS_KEY`) — `sandbox create` prints a non-blocking warning. Matching is on whole segments, so unrelated names like `TOKENIZERS_PARALLELISM` or `PASSWORDLESS_LOGIN` do not warn. 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.
206+
205207
You can also set per-command environment variables with `sandbox exec`:
206208
207209
```shell

0 commit comments

Comments
 (0)