Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion crates/openshell-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,11 @@ enum ProviderRefreshCommands {
#[arg(long = "material", value_name = "KEY=VALUE")]
material: Vec<String>,

/// Secret refresh material resolved from the CLI environment
/// (`ENVVAR` defaults to `KEY`); `KEY` is auto-marked secret.
#[arg(long = "secret-material-env", value_name = "KEY[=ENVVAR]")]
secret_material_env: Vec<String>,

/// Material keys that are secret and must not be exposed.
#[arg(long = "secret-material-key", value_name = "KEY")]
secret_material_keys: Vec<String>,
Expand Down Expand Up @@ -2922,6 +2927,7 @@ async fn main() -> Result<()> {
credential_key,
strategy,
material,
secret_material_env,
secret_material_keys,
credential_expires_at,
} => {
Expand All @@ -2932,6 +2938,7 @@ async fn main() -> Result<()> {
credential_key: &credential_key,
strategy: strategy.as_str(),
material: &material,
secret_material_env: &secret_material_env,
secret_material_keys: &secret_material_keys,
credential_expires_at_ms: credential_expires_at,
},
Expand Down Expand Up @@ -4189,6 +4196,8 @@ mod tests {
"oauth2-client-credentials",
"--material",
"tenant_id=abc",
"--secret-material-env",
"client_secret=GRAPH_CLIENT_SECRET",
"--secret-material-key",
"client_secret",
"--credential-expires-at",
Expand All @@ -4202,10 +4211,11 @@ mod tests {
ProviderRefreshCommands::Configure {
strategy: CliProviderRefreshStrategy::Oauth2ClientCredentials,
credential_expires_at: Some(1_767_225_600_000),
ref secret_material_env,
..
}
))
})
}) if secret_material_env == &["client_secret=GRAPH_CLIENT_SECRET".to_string()]
));

let rotate = Cli::try_parse_from([
Expand Down
138 changes: 131 additions & 7 deletions crates/openshell-cli/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4007,6 +4007,47 @@ pub fn parse_env_pairs(items: &[String]) -> Result<HashMap<String, String>> {
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<HashMap<String, String>> {
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 {
Expand Down Expand Up @@ -5282,6 +5323,7 @@ pub struct ProviderRefreshConfigInput<'a> {
pub credential_key: &'a str,
pub strategy: &'a str,
pub material: &'a [String],
pub secret_material_env: &'a [String],
pub secret_material_keys: &'a [String],
pub credential_expires_at_ms: Option<i64>,
}
Expand All @@ -5292,15 +5334,29 @@ pub async fn provider_refresh_config(
tls: &TlsOptions,
) -> Result<()> {
let strategy = provider_refresh_strategy(input.strategy)?;
let material = parse_key_value_pairs(input.material, "--material")?;
let mut material = parse_key_value_pairs(input.material, "--material")?;
let mut secret_material_keys = input.secret_material_keys.to_vec();
// Env-resolved secrets are auto-marked secret; duplicate keys are an
// error rather than a precedence order.
for (key, value) in parse_secret_material_env_pairs(input.secret_material_env)? {
if material.contains_key(&key) {
return Err(miette!(
"duplicate material key '{key}': supplied via both --material and --secret-material-env"
));
}
if !secret_material_keys.contains(&key) {
secret_material_keys.push(key.clone());
}
material.insert(key, value);
}
let mut client = grpc_client(server, tls).await?;
let status = client
.configure_provider_refresh(ConfigureProviderRefreshRequest {
provider: input.name.to_string(),
credential_key: input.credential_key.to_string(),
strategy: strategy as i32,
material,
secret_material_keys: input.secret_material_keys.to_vec(),
secret_material_keys,
expires_at_ms: input.credential_expires_at_ms,
})
.await
Expand Down Expand Up @@ -7845,11 +7901,12 @@ mod tests {
gateway_type_label, git_sync_files, http_health_check, import_local_package_mtls_bundle,
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, plaintext_gateway_is_remote,
progress_step_from_metadata, 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,
parse_credential_pairs, parse_driver_config_json, parse_secret_material_env_pairs,
plaintext_gateway_is_remote, progress_step_from_metadata,
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 hyper::StatusCode;
Expand Down Expand Up @@ -7993,6 +8050,73 @@ mod tests {
));
}

#[test]
fn parse_secret_material_env_pairs_reads_value_from_named_environment_variable() {
let _guard = EnvVarGuard::set("NAV_PARSE_SME_NAMED", "pem-material");

let parsed =
parse_secret_material_env_pairs(&["private_key=NAV_PARSE_SME_NAMED".to_string()])
.expect("parse");
assert_eq!(parsed.get("private_key"), Some(&"pem-material".to_string()));
}

#[test]
fn parse_secret_material_env_pairs_defaults_env_name_to_key() {
let _guard = EnvVarGuard::set("NAV_PARSE_SME_KEY_ONLY", "key-only-material");

let parsed = parse_secret_material_env_pairs(&["NAV_PARSE_SME_KEY_ONLY".to_string()])
.expect("parse");
assert_eq!(
parsed.get("NAV_PARSE_SME_KEY_ONLY"),
Some(&"key-only-material".to_string())
);
}

#[test]
fn parse_secret_material_env_pairs_rejects_missing_environment() {
let _guard = EnvVarGuard::unset("NAV_PARSE_SME_MISSING");

let err =
parse_secret_material_env_pairs(&["private_key=NAV_PARSE_SME_MISSING".to_string()])
.expect_err("missing env should error");
assert!(err.to_string().contains(
"requires local env var 'NAV_PARSE_SME_MISSING' to be set to a non-empty value"
));
}

#[test]
fn parse_secret_material_env_pairs_rejects_empty_environment_value() {
let _guard = EnvVarGuard::set("NAV_PARSE_SME_EMPTY", " ");

let err = parse_secret_material_env_pairs(&["private_key=NAV_PARSE_SME_EMPTY".to_string()])
.expect_err("blank env should error");
assert!(err.to_string().contains(
"requires local env var 'NAV_PARSE_SME_EMPTY' to be set to a non-empty value"
));
}

#[test]
fn parse_secret_material_env_pairs_rejects_empty_key() {
let err = parse_secret_material_env_pairs(&["=NAV_PARSE_SME_NO_KEY".to_string()])
.expect_err("empty key should error");
assert!(err.to_string().contains("key cannot be empty"));
}

#[test]
fn parse_secret_material_env_pairs_rejects_duplicate_keys() {
let _guard = EnvVarGuard::set("NAV_PARSE_SME_DUP", "value");

let err = parse_secret_material_env_pairs(&[
"private_key=NAV_PARSE_SME_DUP".to_string(),
"private_key=NAV_PARSE_SME_DUP".to_string(),
])
.expect_err("duplicate key should error");
assert!(
err.to_string()
.contains("key 'private_key' supplied more than once")
);
}

#[test]
fn parse_credential_expiry_pairs_accepts_epoch_millis_and_rfc3339() {
let parsed = parse_credential_expiry_pairs(&[
Expand Down
Loading
Loading