From a26a6341af97566c0bc80516975e4c110744035d Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Tue, 9 Jun 2026 09:34:34 +0200 Subject: [PATCH 1/5] feat(cli): add JSON/YAML output format to provider list command Closes #1745 Add -o/--output flag support (json, yaml, table) to openshell provider list by migrating it to use the existing generic output formatter from #1750. This follows the established pattern used by sandbox list, gateway list, and provider list-profiles commands. Changes: - crates/openshell-cli/src/run.rs: Add provider_to_json() helper that maps Provider proto fields to JSON, including only credential keys (not values) for security. Add output parameter to provider_list() and insert early-return for structured formats. - crates/openshell-cli/src/main.rs: Add output field to ProviderCommands::List with conflicts_with constraints between names and output flags. Update invocation to pass output parameter. - crates/openshell-cli/tests/provider_commands_integration.rs: Update provider_list() call to include output parameter. Security: The provider_to_json() helper only exposes credential keys, never credential values, preventing CWE-532 (insertion of sensitive information into output). --- crates/openshell-cli/src/main.rs | 10 +++- crates/openshell-cli/src/run.rs | 59 +++++++++++++++++++ .../tests/provider_commands_integration.rs | 2 +- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index ea0dd79ca..ffa51a43f 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?; diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 85cfaf01c..2ef9500c4 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4778,11 +4778,65 @@ 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 (non-secret configuration) + if !provider.config.is_empty() { + obj.insert("config".to_string(), serde_json::json!(provider.config)); + } + + // 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_ms".to_string(), + serde_json::json!(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 +4846,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."); diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index b287b4ea0..d8d05e3b7 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"); From 509db497b28ffe3c51ea3e394dc80228dbd0437b Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Mon, 15 Jun 2026 15:34:52 +0200 Subject: [PATCH 2/5] refactor(cli): expose only config keys in provider list JSON/YAML output Update provider_to_json() to only include config keys (not values) in structured output, matching the security model used for credentials. This prevents potential exposure of sensitive configuration values. Changed field name from "config" to "config_keys" to clarify that only keys are included, consistent with "credential_keys" field naming. --- crates/openshell-cli/src/run.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 2ef9500c4..392d39d31 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4796,9 +4796,10 @@ fn provider_to_json(provider: &Provider) -> serde_json::Value { serde_json::json!(credential_keys), ); - // Config (non-secret configuration) + // Config keys (keys only, not values) if !provider.config.is_empty() { - obj.insert("config".to_string(), serde_json::json!(provider.config)); + 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) From 703169c35b097c1f44d6481177584559669e5164 Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Mon, 15 Jun 2026 15:45:42 +0200 Subject: [PATCH 3/5] test(cli): add comprehensive tests for provider list JSON/YAML output Add three layers of test coverage for provider list structured output: 1. Unit tests in run.rs for provider_to_json() (8 tests): - Core field validation (id, name, type) - Security: Credential keys exposed, values hidden - Security: Config keys exposed, values hidden - Metadata field inclusion/omission logic - Empty field omission behavior - Credential expiration times 2. CLI parsing tests in main.rs (4 tests): - JSON/YAML output format acceptance - Conflicts between --names and -o flags 3. Integration tests in provider_commands_integration.rs (3 tests): - JSON output execution with populated provider - YAML output execution with populated provider - Empty provider list edge case Security focus: Multiple tests verify that credential and config VALUES are never exposed in JSON/YAML output, only keys. This prevents CWE-532 (insertion of sensitive information into output). All 778 existing unit tests continue to pass, plus 15 new tests added. --- crates/openshell-cli/src/main.rs | 46 +++++ crates/openshell-cli/src/run.rs | 195 ++++++++++++++++++ .../tests/provider_commands_integration.rs | 66 ++++++ 3 files changed, 307 insertions(+) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index ffa51a43f..9b80f1914 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -3883,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 392d39d31..7d81f8674 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -9292,4 +9292,199 @@ 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, + 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_ms"], 1_234_567_890); + 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_ms").is_none(), + "zero created_at_ms 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 + ); + } } diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index d8d05e3b7..1450b99d4 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -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; From a5f59ce97fa275810c10f664789bedf8378d788b Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Mon, 15 Jun 2026 16:01:28 +0200 Subject: [PATCH 4/5] feat(cli): format created_at as human-readable datetime in provider list JSON/YAML MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update provider_to_json() to format created_at_ms as a human-readable datetime string using format_epoch_ms(), matching the pattern used by sandbox_to_json(). Changes: - Field name: created_at_ms → created_at - Value format: Raw milliseconds → "YYYY-MM-DD HH:MM:SS" - Implementation: Use format_epoch_ms(meta.created_at_ms) This change: - Aligns provider list output with sandbox list output for consistency - Makes JSON/YAML output more human-readable for interactive use - Follows the established pattern across all *_to_json() functions Breaking change: Field renamed and type changed (number → string). However, this feature was just added in PR #1830 and hasn't been released yet, so no users are affected. Example output: Before: {"created_at_ms": 1234567890000} After: {"created_at": "2009-02-13 23:31:30"} Tests updated: - provider_to_json_includes_metadata_fields_when_present: Assert formatted string - provider_to_json_omits_zero_metadata_fields: Update field name - provider_to_json_formats_created_at_as_human_readable: New test validating format --- crates/openshell-cli/src/run.rs | 39 ++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 7d81f8674..6f5520755 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4815,8 +4815,8 @@ fn provider_to_json(provider: &Provider) -> serde_json::Value { } if meta.created_at_ms != 0 { obj.insert( - "created_at_ms".to_string(), - serde_json::json!(meta.created_at_ms), + "created_at".to_string(), + serde_json::json!(format_epoch_ms(meta.created_at_ms)), ); } } @@ -9414,7 +9414,7 @@ mod tests { id: "prov-123".to_string(), name: "test-provider".to_string(), resource_version: 42, - created_at_ms: 1_234_567_890, + created_at_ms: 1_234_567_890_000, labels, }; @@ -9429,7 +9429,7 @@ mod tests { let json = super::provider_to_json(&provider); assert_eq!(json["resource_version"], 42); - assert_eq!(json["created_at_ms"], 1_234_567_890); + assert_eq!(json["created_at"], "2009-02-13 23:31:30"); assert_eq!(json["labels"]["env"], "prod"); } @@ -9458,8 +9458,8 @@ mod tests { "zero resource_version should be omitted" ); assert!( - json.get("created_at_ms").is_none(), - "zero created_at_ms should be omitted" + json.get("created_at").is_none(), + "zero created_at should be omitted" ); assert!( json.get("labels").is_none(), @@ -9487,4 +9487,31 @@ mod tests { 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" + ); + } } From 993e557e691a0c454bd19b3491af5cdb2fb21d61 Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Mon, 15 Jun 2026 18:17:55 +0200 Subject: [PATCH 5/5] docs(cli): document JSON/YAML output format for provider list Add documentation for the -o/--output flag on provider list command, following the established pattern from sandbox list documentation. Changes: - Add examples showing -o json and -o yaml usage - Explain output structure (metadata, credential keys, config keys) - Emphasize security design (keys-only exposure, never values) - Cross-reference existing Credential Injection section The documentation matches the feature added in a26a6341 and refined in subsequent commits (509db497, a5f59ce9). Signed-off-by: Jeff MAURY --- docs/sandboxes/manage-providers.mdx | 9 +++++++++ 1 file changed, 9 insertions(+) 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