feat(cli): add --secret-material-env to provider refresh configure

hunglp6d · hunglp6d · commit cc785bef5081 · 2026-07-08T16:06:33.000+05:30
diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs
@@ -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>,
@@ -2922,6 +2927,7 @@ async fn main() -> Result<()> {
                         credential_key,
                         strategy,
                         material,
+                        secret_material_env,
                         secret_material_keys,
                         credential_expires_at,
                     } => {
@@ -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,
                             },
@@ -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",
@@ -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([
diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs
@@ -4007,6 +4007,42 @@ 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"
+            ));
+        }
+
+        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 {
@@ -5282,6 +5318,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>,
 }
@@ -5292,15 +5329,23 @@ 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 override `--material` duplicates and are auto-marked secret.
+    for (key, value) in parse_secret_material_env_pairs(input.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
@@ -7845,11 +7890,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;
@@ -7993,6 +8039,58 @@ 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_credential_expiry_pairs_accepts_epoch_millis_and_rfc3339() {
         let parsed = parse_credential_expiry_pairs(&[
diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs
@@ -62,6 +62,8 @@ enum ProviderRefreshRequestLog {
     Configure {
         provider_name: String,
         credential_key: String,
+        material: HashMap<String, String>,
+        secret_material_keys: Vec<String>,
         expires_at_ms: Option<i64>,
     },
     Rotate {
@@ -661,6 +663,8 @@ impl OpenShell for TestOpenShell {
             .push(ProviderRefreshRequestLog::Configure {
                 provider_name: request.provider.clone(),
                 credential_key: request.credential_key.clone(),
+                material: request.material.clone(),
+                secret_material_keys: request.secret_material_keys.clone(),
                 expires_at_ms: request.expires_at_ms,
             });
         let configure_failure = self
@@ -1187,6 +1191,7 @@ async fn provider_refresh_cli_run_functions_wire_requests() {
             credential_key: "MS_GRAPH_ACCESS_TOKEN",
             strategy: "oauth2_client_credentials",
             material: &["tenant_id=tenant".to_string()],
+            secret_material_env: &[],
             secret_material_keys: &["client_secret".to_string()],
             credential_expires_at_ms: Some(1_767_225_600_000),
         },
@@ -1216,6 +1221,8 @@ async fn provider_refresh_cli_run_functions_wire_requests() {
             ProviderRefreshRequestLog::Configure {
                 provider_name: "my-graph".to_string(),
                 credential_key: "MS_GRAPH_ACCESS_TOKEN".to_string(),
+                material: HashMap::from([("tenant_id".to_string(), "tenant".to_string())]),
+                secret_material_keys: vec!["client_secret".to_string()],
                 expires_at_ms: Some(1_767_225_600_000),
             },
             ProviderRefreshRequestLog::Status {
@@ -1234,6 +1241,91 @@ async fn provider_refresh_cli_run_functions_wire_requests() {
     );
 }
 
+#[tokio::test]
+async fn provider_refresh_configure_reads_secret_material_from_env_off_argv() {
+    let ts = run_server().await;
+
+    run::provider_create(
+        &ts.endpoint,
+        "gc-bridge",
+        "outlook",
+        false,
+        &["GOOGLE_CHAT_ACCESS_TOKEN=pending".to_string()],
+        false,
+        &[],
+        &ts.tls,
+    )
+    .await
+    .expect("provider create");
+
+    // The env value overrides the `--material` duplicate and is auto-marked secret.
+    let guard = EnvVarGuard::set(&[("OPENSHELL_ITEST_SME_PRIVATE_KEY", "pem-from-env")]);
+    run::provider_refresh_config(
+        &ts.endpoint,
+        run::ProviderRefreshConfigInput {
+            name: "gc-bridge",
+            credential_key: "GOOGLE_CHAT_ACCESS_TOKEN",
+            strategy: "google_service_account_jwt",
+            material: &[
+                "client_email=bot@p.iam.gserviceaccount.com".to_string(),
+                "private_key=argv-value-must-be-overridden".to_string(),
+            ],
+            secret_material_env: &["private_key=OPENSHELL_ITEST_SME_PRIVATE_KEY".to_string()],
+            secret_material_keys: &[],
+            credential_expires_at_ms: None,
+        },
+        &ts.tls,
+    )
+    .await
+    .expect("provider refresh configure");
+    drop(guard);
+
+    let requests = ts.state.refresh_requests.lock().await.clone();
+    assert_eq!(
+        requests,
+        vec![ProviderRefreshRequestLog::Configure {
+            provider_name: "gc-bridge".to_string(),
+            credential_key: "GOOGLE_CHAT_ACCESS_TOKEN".to_string(),
+            material: HashMap::from([
+                (
+                    "client_email".to_string(),
+                    "bot@p.iam.gserviceaccount.com".to_string()
+                ),
+                ("private_key".to_string(), "pem-from-env".to_string()),
+            ]),
+            secret_material_keys: vec!["private_key".to_string()],
+            expires_at_ms: None,
+        }]
+    );
+}
+
+#[tokio::test]
+async fn provider_refresh_configure_fails_closed_when_secret_material_env_is_unset() {
+    let ts = run_server().await;
+
+    let err = run::provider_refresh_config(
+        &ts.endpoint,
+        run::ProviderRefreshConfigInput {
+            name: "gc-bridge",
+            credential_key: "GOOGLE_CHAT_ACCESS_TOKEN",
+            strategy: "google_service_account_jwt",
+            material: &[],
+            secret_material_env: &["private_key=OPENSHELL_ITEST_SME_DEFINITELY_UNSET".to_string()],
+            secret_material_keys: &[],
+            credential_expires_at_ms: None,
+        },
+        &ts.tls,
+    )
+    .await
+    .expect_err("unset env should fail before any request is sent");
+
+    assert!(err.to_string().contains(
+        "requires local env var 'OPENSHELL_ITEST_SME_DEFINITELY_UNSET' to be set to a non-empty value"
+    ));
+    // Fails closed on the client side: nothing reached the gateway.
+    assert!(ts.state.refresh_requests.lock().await.is_empty());
+}
+
 #[tokio::test]
 async fn provider_create_allows_empty_credentials_for_gateway_refresh_profiles() {
     let ts = run_server().await;
@@ -2197,14 +2289,15 @@ async fn provider_create_from_gcloud_adc_happy_path() {
         2,
         "expected configure + rotate refresh requests"
     );
-    assert_eq!(
-        requests[0],
+    assert!(matches!(
+        &requests[0],
         ProviderRefreshRequestLog::Configure {
-            provider_name: "my-vertex".to_string(),
-            credential_key: "GOOGLE_VERTEX_AI_TOKEN".to_string(),
+            provider_name,
+            credential_key,
             expires_at_ms: None,
-        }
-    );
+            ..
+        } if provider_name == "my-vertex" && credential_key == "GOOGLE_VERTEX_AI_TOKEN"
+    ));
     assert_eq!(
         requests[1],
         ProviderRefreshRequestLog::Rotate {
@@ -2577,14 +2670,15 @@ async fn provider_create_from_gcloud_adc_with_config_keys() {
         2,
         "exactly one configure call and one rotate call expected"
     );
-    assert_eq!(
-        refresh_requests[0],
+    assert!(matches!(
+        &refresh_requests[0],
         ProviderRefreshRequestLog::Configure {
-            provider_name: "vertex-with-config".to_string(),
-            credential_key: "GOOGLE_VERTEX_AI_TOKEN".to_string(),
+            provider_name,
+            credential_key,
             expires_at_ms: None,
-        }
-    );
+            ..
+        } if provider_name == "vertex-with-config" && credential_key == "GOOGLE_VERTEX_AI_TOKEN"
+    ));
     assert_eq!(
         refresh_requests[1],
         ProviderRefreshRequestLog::Rotate {
diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx
@@ -147,11 +147,17 @@ openshell provider refresh configure my-graph \
   --strategy oauth2-client-credentials \
   --material tenant_id="$TENANT_ID" \
   --material client_id="$CLIENT_ID" \
-  --material client_secret="$CLIENT_SECRET" \
-  --secret-material-key client_secret \
+  --secret-material-env client_secret=CLIENT_SECRET \
   --credential-expires-at 1767225600000
 ```
 
+Pass secret material with `--secret-material-env KEY[=ENVVAR]` (`ENVVAR`
+defaults to `KEY`): the CLI reads the value from its own environment, so the
+secret never appears in the host process table the way an expanded
+`--material KEY="$VALUE"` argument would, and `KEY` is automatically marked
+secret. Keep non-secret material on `--material`; `--secret-material-key KEY`
+still marks a key supplied through `--material` as secret.
+
 Check refresh status:
 
 ```shell
