diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index ea0dd79ca..9b80f1914 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -777,8 +777,12 @@ enum ProviderCommands { offset: u32, /// Print only provider names, one per line. - #[arg(long)] + #[arg(long, conflicts_with = "output")] names: bool, + + /// Output format. + #[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table, conflicts_with = "names")] + output: OutputFormat, }, /// List available provider profiles. @@ -2902,8 +2906,10 @@ async fn main() -> Result<()> { limit, offset, names, + output, } => { - run::provider_list(endpoint, limit, offset, names, &tls).await?; + run::provider_list(endpoint, limit, offset, names, output.as_str(), &tls) + .await?; } ProviderCommands::ListProfiles { output } => { run::provider_list_profiles(endpoint, output.as_str(), &tls).await?; @@ -3877,6 +3883,52 @@ mod tests { )); } + #[test] + fn provider_list_accepts_output_json() { + let cli = Cli::try_parse_from(["openshell", "provider", "list", "-o", "json"]) + .expect("provider list -o json should parse"); + + assert!(matches!( + cli.command, + Some(Commands::Provider { + command: Some(ProviderCommands::List { + output: OutputFormat::Json, + .. + }) + }) + )); + } + + #[test] + fn provider_list_accepts_output_yaml() { + let cli = Cli::try_parse_from(["openshell", "provider", "list", "-o", "yaml"]) + .expect("provider list -o yaml should parse"); + + assert!(matches!( + cli.command, + Some(Commands::Provider { + command: Some(ProviderCommands::List { + output: OutputFormat::Yaml, + .. + }) + }) + )); + } + + #[test] + fn provider_list_output_conflicts_with_names() { + let result = + Cli::try_parse_from(["openshell", "provider", "list", "-o", "json", "--names"]); + assert!(result.is_err(), "--names and -o should conflict"); + } + + #[test] + fn provider_list_names_conflicts_with_output() { + let result = + Cli::try_parse_from(["openshell", "provider", "list", "--names", "-o", "yaml"]); + assert!(result.is_err(), "--names and -o should conflict"); + } + #[test] fn provider_create_accepts_custom_profile_type_ids() { let cli = Cli::try_parse_from([ diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 85cfaf01c..6f5520755 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4778,11 +4778,66 @@ pub async fn provider_get(server: &str, name: &str, tls: &TlsOptions) -> Result< Ok(()) } +fn provider_to_json(provider: &Provider) -> serde_json::Value { + let mut obj = serde_json::Map::new(); + + // Core fields + obj.insert("id".to_string(), serde_json::json!(provider.object_id())); + obj.insert( + "name".to_string(), + serde_json::json!(provider.object_name()), + ); + obj.insert("type".to_string(), serde_json::json!(provider.r#type)); + + // Credential keys (NEVER values - security) + let credential_keys: Vec = provider.credentials.keys().cloned().collect(); + obj.insert( + "credential_keys".to_string(), + serde_json::json!(credential_keys), + ); + + // Config keys (keys only, not values) + if !provider.config.is_empty() { + let config_keys: Vec = provider.config.keys().cloned().collect(); + obj.insert("config_keys".to_string(), serde_json::json!(config_keys)); + } + + // Metadata fields (only if metadata exists) + if let Some(meta) = &provider.metadata { + if !meta.labels.is_empty() { + obj.insert("labels".to_string(), serde_json::json!(meta.labels)); + } + if meta.resource_version != 0 { + obj.insert( + "resource_version".to_string(), + serde_json::json!(meta.resource_version), + ); + } + if meta.created_at_ms != 0 { + obj.insert( + "created_at".to_string(), + serde_json::json!(format_epoch_ms(meta.created_at_ms)), + ); + } + } + + // Credential expiration times (only if present) + if !provider.credential_expires_at_ms.is_empty() { + obj.insert( + "credential_expires_at_ms".to_string(), + serde_json::json!(provider.credential_expires_at_ms), + ); + } + + serde_json::Value::Object(obj) +} + pub async fn provider_list( server: &str, limit: u32, offset: u32, names_only: bool, + output: &str, tls: &TlsOptions, ) -> Result<()> { let mut client = grpc_client(server, tls).await?; @@ -4792,6 +4847,11 @@ pub async fn provider_list( .into_diagnostic()?; let providers = response.into_inner().providers; + // Handle structured output formats (json, yaml) + if crate::output::print_output_collection(output, &providers, provider_to_json)? { + return Ok(()); + } + if providers.is_empty() { if !names_only { println!("No providers found."); @@ -9232,4 +9292,226 @@ mod tests { assert_eq!(load_active_gateway().as_deref(), Some("system-default")); }); } + + #[test] + fn provider_to_json_includes_core_fields() { + let metadata = ObjectMeta { + id: "prov-123".to_string(), + name: "test-provider".to_string(), + ..Default::default() + }; + + let provider = Provider { + metadata: Some(metadata), + r#type: "anthropic".to_string(), + credentials: std::collections::HashMap::new(), + config: std::collections::HashMap::new(), + credential_expires_at_ms: std::collections::HashMap::new(), + }; + + let json = super::provider_to_json(&provider); + + assert_eq!(json["id"], "prov-123"); + assert_eq!(json["name"], "test-provider"); + assert_eq!(json["type"], "anthropic"); + } + + #[test] + fn provider_to_json_exposes_credential_keys_not_values() { + let mut credentials = std::collections::HashMap::new(); + credentials.insert("ANTHROPIC_API_KEY".to_string(), "secret-value".to_string()); + credentials.insert("OTHER_KEY".to_string(), "other-secret".to_string()); + + let provider = Provider { + metadata: Some(ObjectMeta::default()), + r#type: "anthropic".to_string(), + credentials, + config: std::collections::HashMap::new(), + credential_expires_at_ms: std::collections::HashMap::new(), + }; + + let json = super::provider_to_json(&provider); + let json_str = json.to_string(); + + // Assert credential keys are present + let keys = json["credential_keys"].as_array().unwrap(); + assert_eq!(keys.len(), 2); + assert!(keys.iter().any(|k| k.as_str() == Some("ANTHROPIC_API_KEY"))); + assert!(keys.iter().any(|k| k.as_str() == Some("OTHER_KEY"))); + + // Assert credential values are NOT in the output (SECURITY) + assert!( + !json_str.contains("secret-value"), + "credential values must not be exposed" + ); + assert!( + !json_str.contains("other-secret"), + "credential values must not be exposed" + ); + } + + #[test] + fn provider_to_json_exposes_config_keys_not_values() { + let mut config = std::collections::HashMap::new(); + config.insert("region".to_string(), "us-west".to_string()); + config.insert( + "endpoint".to_string(), + "https://api.example.com".to_string(), + ); + + let provider = Provider { + metadata: Some(ObjectMeta::default()), + r#type: "custom".to_string(), + credentials: std::collections::HashMap::new(), + config, + credential_expires_at_ms: std::collections::HashMap::new(), + }; + + let json = super::provider_to_json(&provider); + let json_str = json.to_string(); + + // Assert config keys are present + let keys = json["config_keys"].as_array().unwrap(); + assert_eq!(keys.len(), 2); + assert!(keys.iter().any(|k| k.as_str() == Some("region"))); + assert!(keys.iter().any(|k| k.as_str() == Some("endpoint"))); + + // Assert config values are NOT in the output (SECURITY) + assert!( + !json_str.contains("us-west"), + "config values must not be exposed" + ); + assert!( + !json_str.contains("https://api.example.com"), + "config values must not be exposed" + ); + } + + #[test] + fn provider_to_json_omits_empty_config() { + let provider = Provider { + metadata: Some(ObjectMeta::default()), + r#type: "anthropic".to_string(), + credentials: std::collections::HashMap::new(), + config: std::collections::HashMap::new(), // Empty config + credential_expires_at_ms: std::collections::HashMap::new(), + }; + + let json = super::provider_to_json(&provider); + + assert!( + json.get("config_keys").is_none(), + "empty config_keys should be omitted" + ); + } + + #[test] + fn provider_to_json_includes_metadata_fields_when_present() { + let mut labels = std::collections::HashMap::new(); + labels.insert("env".to_string(), "prod".to_string()); + + let metadata = ObjectMeta { + id: "prov-123".to_string(), + name: "test-provider".to_string(), + resource_version: 42, + created_at_ms: 1_234_567_890_000, + labels, + }; + + let provider = Provider { + metadata: Some(metadata), + r#type: "anthropic".to_string(), + credentials: std::collections::HashMap::new(), + config: std::collections::HashMap::new(), + credential_expires_at_ms: std::collections::HashMap::new(), + }; + + let json = super::provider_to_json(&provider); + + assert_eq!(json["resource_version"], 42); + assert_eq!(json["created_at"], "2009-02-13 23:31:30"); + assert_eq!(json["labels"]["env"], "prod"); + } + + #[test] + fn provider_to_json_omits_zero_metadata_fields() { + let metadata = ObjectMeta { + id: "prov-123".to_string(), + name: "test-provider".to_string(), + // resource_version and created_at_ms are 0 + // labels is empty + ..Default::default() + }; + + let provider = Provider { + metadata: Some(metadata), + r#type: "anthropic".to_string(), + credentials: std::collections::HashMap::new(), + config: std::collections::HashMap::new(), + credential_expires_at_ms: std::collections::HashMap::new(), + }; + + let json = super::provider_to_json(&provider); + + assert!( + json.get("resource_version").is_none(), + "zero resource_version should be omitted" + ); + assert!( + json.get("created_at").is_none(), + "zero created_at should be omitted" + ); + assert!( + json.get("labels").is_none(), + "empty labels should be omitted" + ); + } + + #[test] + fn provider_to_json_includes_credential_expiration() { + let mut credential_expires_at_ms = std::collections::HashMap::new(); + credential_expires_at_ms.insert("ACCESS_TOKEN".to_string(), 1_234_567_890); + + let provider = Provider { + metadata: Some(ObjectMeta::default()), + r#type: "oauth".to_string(), + credentials: std::collections::HashMap::new(), + config: std::collections::HashMap::new(), + credential_expires_at_ms, + }; + + let json = super::provider_to_json(&provider); + + assert_eq!( + json["credential_expires_at_ms"]["ACCESS_TOKEN"], + 1_234_567_890 + ); + } + + #[test] + fn provider_to_json_formats_created_at_as_human_readable() { + let metadata = ObjectMeta { + id: "prov-123".to_string(), + name: "test-provider".to_string(), + created_at_ms: 1_609_459_200_000, // 2021-01-01 00:00:00 + ..Default::default() + }; + + let provider = Provider { + metadata: Some(metadata), + r#type: "anthropic".to_string(), + credentials: std::collections::HashMap::new(), + config: std::collections::HashMap::new(), + credential_expires_at_ms: std::collections::HashMap::new(), + }; + + let json = super::provider_to_json(&provider); + + // Should format as human-readable datetime, not raw milliseconds + assert_eq!(json["created_at"], "2021-01-01 00:00:00"); + assert!( + json.get("created_at_ms").is_none(), + "raw milliseconds field should not exist" + ); + } } diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index b287b4ea0..1450b99d4 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -997,7 +997,7 @@ async fn provider_cli_run_functions_support_full_crud_flow() { run::provider_get(&ts.endpoint, "my-claude", &ts.tls) .await .expect("provider get"); - run::provider_list(&ts.endpoint, 100, 0, false, &ts.tls) + run::provider_list(&ts.endpoint, 100, 0, false, "table", &ts.tls) .await .expect("provider list"); @@ -1027,6 +1027,72 @@ async fn provider_list_profiles_cli_uses_profile_browsing_rpc() { .expect("provider list-profiles"); } +#[tokio::test] +async fn provider_list_json_output() { + let ts = run_server().await; + + // Create a provider with credentials and config + run::provider_create( + &ts.endpoint, + "test-provider", + "anthropic", + false, + &["ANTHROPIC_API_KEY=test-key".to_string()], + false, + &["region=us-west".to_string()], + &ts.tls, + ) + .await + .expect("provider create"); + + // Test JSON output (verifies it doesn't error) + run::provider_list(&ts.endpoint, 100, 0, false, "json", &ts.tls) + .await + .expect("provider list json should succeed"); + + run::provider_delete(&ts.endpoint, &["test-provider".to_string()], &ts.tls) + .await + .expect("provider delete"); +} + +#[tokio::test] +async fn provider_list_yaml_output() { + let ts = run_server().await; + + // Create a provider with credentials and config + run::provider_create( + &ts.endpoint, + "test-provider", + "anthropic", + false, + &["ANTHROPIC_API_KEY=test-key".to_string()], + false, + &["region=us-west".to_string()], + &ts.tls, + ) + .await + .expect("provider create"); + + // Test YAML output (verifies it doesn't error) + run::provider_list(&ts.endpoint, 100, 0, false, "yaml", &ts.tls) + .await + .expect("provider list yaml should succeed"); + + run::provider_delete(&ts.endpoint, &["test-provider".to_string()], &ts.tls) + .await + .expect("provider delete"); +} + +#[tokio::test] +async fn provider_list_json_empty() { + let ts = run_server().await; + + // Test JSON output with no providers (verifies it doesn't error on empty list) + run::provider_list(&ts.endpoint, 100, 0, false, "json", &ts.tls) + .await + .expect("provider list json empty should succeed"); +} + #[tokio::test] async fn provider_refresh_cli_run_functions_wire_requests() { let ts = run_server().await; diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index a6b9654d0..67fa8f23a 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -85,6 +85,15 @@ List all providers: openshell provider list ``` +Use `-o json` or `-o yaml` for machine-readable output: + +```shell +openshell provider list -o json +openshell provider list -o yaml +``` + +Structured output includes provider metadata (`id`, `name`, `type`), credential key names, config key names, labels, creation timestamp, resource version, and credential expiration times. Only credential and config keys are exposed, never values, preventing accidental credential leakage in logs or output. For details on how credentials are injected into sandboxes, refer to [Credential Injection](#how-credential-injection-works). + Inspect a provider: ```shell