Skip to content

Commit ed65bfd

Browse files
authored
feat(cli): add JSON/YAML output format to provider list command (NVIDIA#1830)
* feat(cli): add JSON/YAML output format to provider list command Closes NVIDIA#1745 Add -o/--output flag support (json, yaml, table) to openshell provider list by migrating it to use the existing generic output formatter from NVIDIA#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). * 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. * 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. * feat(cli): format created_at as human-readable datetime in provider list JSON/YAML 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 NVIDIA#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 * 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 a26a634 and refined in subsequent commits (509db49, a5f59ce). Signed-off-by: Jeff MAURY <jmaury@redhat.com> --------- Signed-off-by: Jeff MAURY <jmaury@redhat.com>
1 parent 1ca23bc commit ed65bfd

4 files changed

Lines changed: 412 additions & 3 deletions

File tree

‎crates/openshell-cli/src/main.rs‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -777,8 +777,12 @@ enum ProviderCommands {
777777
offset: u32,
778778

779779
/// Print only provider names, one per line.
780-
#[arg(long)]
780+
#[arg(long, conflicts_with = "output")]
781781
names: bool,
782+
783+
/// Output format.
784+
#[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table, conflicts_with = "names")]
785+
output: OutputFormat,
782786
},
783787

784788
/// List available provider profiles.
@@ -2902,8 +2906,10 @@ async fn main() -> Result<()> {
29022906
limit,
29032907
offset,
29042908
names,
2909+
output,
29052910
} => {
2906-
run::provider_list(endpoint, limit, offset, names, &tls).await?;
2911+
run::provider_list(endpoint, limit, offset, names, output.as_str(), &tls)
2912+
.await?;
29072913
}
29082914
ProviderCommands::ListProfiles { output } => {
29092915
run::provider_list_profiles(endpoint, output.as_str(), &tls).await?;
@@ -3877,6 +3883,52 @@ mod tests {
38773883
));
38783884
}
38793885

3886+
#[test]
3887+
fn provider_list_accepts_output_json() {
3888+
let cli = Cli::try_parse_from(["openshell", "provider", "list", "-o", "json"])
3889+
.expect("provider list -o json should parse");
3890+
3891+
assert!(matches!(
3892+
cli.command,
3893+
Some(Commands::Provider {
3894+
command: Some(ProviderCommands::List {
3895+
output: OutputFormat::Json,
3896+
..
3897+
})
3898+
})
3899+
));
3900+
}
3901+
3902+
#[test]
3903+
fn provider_list_accepts_output_yaml() {
3904+
let cli = Cli::try_parse_from(["openshell", "provider", "list", "-o", "yaml"])
3905+
.expect("provider list -o yaml should parse");
3906+
3907+
assert!(matches!(
3908+
cli.command,
3909+
Some(Commands::Provider {
3910+
command: Some(ProviderCommands::List {
3911+
output: OutputFormat::Yaml,
3912+
..
3913+
})
3914+
})
3915+
));
3916+
}
3917+
3918+
#[test]
3919+
fn provider_list_output_conflicts_with_names() {
3920+
let result =
3921+
Cli::try_parse_from(["openshell", "provider", "list", "-o", "json", "--names"]);
3922+
assert!(result.is_err(), "--names and -o should conflict");
3923+
}
3924+
3925+
#[test]
3926+
fn provider_list_names_conflicts_with_output() {
3927+
let result =
3928+
Cli::try_parse_from(["openshell", "provider", "list", "--names", "-o", "yaml"]);
3929+
assert!(result.is_err(), "--names and -o should conflict");
3930+
}
3931+
38803932
#[test]
38813933
fn provider_create_accepts_custom_profile_type_ids() {
38823934
let cli = Cli::try_parse_from([

‎crates/openshell-cli/src/run.rs‎

Lines changed: 282 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4778,11 +4778,66 @@ pub async fn provider_get(server: &str, name: &str, tls: &TlsOptions) -> Result<
47784778
Ok(())
47794779
}
47804780

4781+
fn provider_to_json(provider: &Provider) -> serde_json::Value {
4782+
let mut obj = serde_json::Map::new();
4783+
4784+
// Core fields
4785+
obj.insert("id".to_string(), serde_json::json!(provider.object_id()));
4786+
obj.insert(
4787+
"name".to_string(),
4788+
serde_json::json!(provider.object_name()),
4789+
);
4790+
obj.insert("type".to_string(), serde_json::json!(provider.r#type));
4791+
4792+
// Credential keys (NEVER values - security)
4793+
let credential_keys: Vec<String> = provider.credentials.keys().cloned().collect();
4794+
obj.insert(
4795+
"credential_keys".to_string(),
4796+
serde_json::json!(credential_keys),
4797+
);
4798+
4799+
// Config keys (keys only, not values)
4800+
if !provider.config.is_empty() {
4801+
let config_keys: Vec<String> = provider.config.keys().cloned().collect();
4802+
obj.insert("config_keys".to_string(), serde_json::json!(config_keys));
4803+
}
4804+
4805+
// Metadata fields (only if metadata exists)
4806+
if let Some(meta) = &provider.metadata {
4807+
if !meta.labels.is_empty() {
4808+
obj.insert("labels".to_string(), serde_json::json!(meta.labels));
4809+
}
4810+
if meta.resource_version != 0 {
4811+
obj.insert(
4812+
"resource_version".to_string(),
4813+
serde_json::json!(meta.resource_version),
4814+
);
4815+
}
4816+
if meta.created_at_ms != 0 {
4817+
obj.insert(
4818+
"created_at".to_string(),
4819+
serde_json::json!(format_epoch_ms(meta.created_at_ms)),
4820+
);
4821+
}
4822+
}
4823+
4824+
// Credential expiration times (only if present)
4825+
if !provider.credential_expires_at_ms.is_empty() {
4826+
obj.insert(
4827+
"credential_expires_at_ms".to_string(),
4828+
serde_json::json!(provider.credential_expires_at_ms),
4829+
);
4830+
}
4831+
4832+
serde_json::Value::Object(obj)
4833+
}
4834+
47814835
pub async fn provider_list(
47824836
server: &str,
47834837
limit: u32,
47844838
offset: u32,
47854839
names_only: bool,
4840+
output: &str,
47864841
tls: &TlsOptions,
47874842
) -> Result<()> {
47884843
let mut client = grpc_client(server, tls).await?;
@@ -4792,6 +4847,11 @@ pub async fn provider_list(
47924847
.into_diagnostic()?;
47934848
let providers = response.into_inner().providers;
47944849

4850+
// Handle structured output formats (json, yaml)
4851+
if crate::output::print_output_collection(output, &providers, provider_to_json)? {
4852+
return Ok(());
4853+
}
4854+
47954855
if providers.is_empty() {
47964856
if !names_only {
47974857
println!("No providers found.");
@@ -9232,4 +9292,226 @@ mod tests {
92329292
assert_eq!(load_active_gateway().as_deref(), Some("system-default"));
92339293
});
92349294
}
9295+
9296+
#[test]
9297+
fn provider_to_json_includes_core_fields() {
9298+
let metadata = ObjectMeta {
9299+
id: "prov-123".to_string(),
9300+
name: "test-provider".to_string(),
9301+
..Default::default()
9302+
};
9303+
9304+
let provider = Provider {
9305+
metadata: Some(metadata),
9306+
r#type: "anthropic".to_string(),
9307+
credentials: std::collections::HashMap::new(),
9308+
config: std::collections::HashMap::new(),
9309+
credential_expires_at_ms: std::collections::HashMap::new(),
9310+
};
9311+
9312+
let json = super::provider_to_json(&provider);
9313+
9314+
assert_eq!(json["id"], "prov-123");
9315+
assert_eq!(json["name"], "test-provider");
9316+
assert_eq!(json["type"], "anthropic");
9317+
}
9318+
9319+
#[test]
9320+
fn provider_to_json_exposes_credential_keys_not_values() {
9321+
let mut credentials = std::collections::HashMap::new();
9322+
credentials.insert("ANTHROPIC_API_KEY".to_string(), "secret-value".to_string());
9323+
credentials.insert("OTHER_KEY".to_string(), "other-secret".to_string());
9324+
9325+
let provider = Provider {
9326+
metadata: Some(ObjectMeta::default()),
9327+
r#type: "anthropic".to_string(),
9328+
credentials,
9329+
config: std::collections::HashMap::new(),
9330+
credential_expires_at_ms: std::collections::HashMap::new(),
9331+
};
9332+
9333+
let json = super::provider_to_json(&provider);
9334+
let json_str = json.to_string();
9335+
9336+
// Assert credential keys are present
9337+
let keys = json["credential_keys"].as_array().unwrap();
9338+
assert_eq!(keys.len(), 2);
9339+
assert!(keys.iter().any(|k| k.as_str() == Some("ANTHROPIC_API_KEY")));
9340+
assert!(keys.iter().any(|k| k.as_str() == Some("OTHER_KEY")));
9341+
9342+
// Assert credential values are NOT in the output (SECURITY)
9343+
assert!(
9344+
!json_str.contains("secret-value"),
9345+
"credential values must not be exposed"
9346+
);
9347+
assert!(
9348+
!json_str.contains("other-secret"),
9349+
"credential values must not be exposed"
9350+
);
9351+
}
9352+
9353+
#[test]
9354+
fn provider_to_json_exposes_config_keys_not_values() {
9355+
let mut config = std::collections::HashMap::new();
9356+
config.insert("region".to_string(), "us-west".to_string());
9357+
config.insert(
9358+
"endpoint".to_string(),
9359+
"https://api.example.com".to_string(),
9360+
);
9361+
9362+
let provider = Provider {
9363+
metadata: Some(ObjectMeta::default()),
9364+
r#type: "custom".to_string(),
9365+
credentials: std::collections::HashMap::new(),
9366+
config,
9367+
credential_expires_at_ms: std::collections::HashMap::new(),
9368+
};
9369+
9370+
let json = super::provider_to_json(&provider);
9371+
let json_str = json.to_string();
9372+
9373+
// Assert config keys are present
9374+
let keys = json["config_keys"].as_array().unwrap();
9375+
assert_eq!(keys.len(), 2);
9376+
assert!(keys.iter().any(|k| k.as_str() == Some("region")));
9377+
assert!(keys.iter().any(|k| k.as_str() == Some("endpoint")));
9378+
9379+
// Assert config values are NOT in the output (SECURITY)
9380+
assert!(
9381+
!json_str.contains("us-west"),
9382+
"config values must not be exposed"
9383+
);
9384+
assert!(
9385+
!json_str.contains("https://api.example.com"),
9386+
"config values must not be exposed"
9387+
);
9388+
}
9389+
9390+
#[test]
9391+
fn provider_to_json_omits_empty_config() {
9392+
let provider = Provider {
9393+
metadata: Some(ObjectMeta::default()),
9394+
r#type: "anthropic".to_string(),
9395+
credentials: std::collections::HashMap::new(),
9396+
config: std::collections::HashMap::new(), // Empty config
9397+
credential_expires_at_ms: std::collections::HashMap::new(),
9398+
};
9399+
9400+
let json = super::provider_to_json(&provider);
9401+
9402+
assert!(
9403+
json.get("config_keys").is_none(),
9404+
"empty config_keys should be omitted"
9405+
);
9406+
}
9407+
9408+
#[test]
9409+
fn provider_to_json_includes_metadata_fields_when_present() {
9410+
let mut labels = std::collections::HashMap::new();
9411+
labels.insert("env".to_string(), "prod".to_string());
9412+
9413+
let metadata = ObjectMeta {
9414+
id: "prov-123".to_string(),
9415+
name: "test-provider".to_string(),
9416+
resource_version: 42,
9417+
created_at_ms: 1_234_567_890_000,
9418+
labels,
9419+
};
9420+
9421+
let provider = Provider {
9422+
metadata: Some(metadata),
9423+
r#type: "anthropic".to_string(),
9424+
credentials: std::collections::HashMap::new(),
9425+
config: std::collections::HashMap::new(),
9426+
credential_expires_at_ms: std::collections::HashMap::new(),
9427+
};
9428+
9429+
let json = super::provider_to_json(&provider);
9430+
9431+
assert_eq!(json["resource_version"], 42);
9432+
assert_eq!(json["created_at"], "2009-02-13 23:31:30");
9433+
assert_eq!(json["labels"]["env"], "prod");
9434+
}
9435+
9436+
#[test]
9437+
fn provider_to_json_omits_zero_metadata_fields() {
9438+
let metadata = ObjectMeta {
9439+
id: "prov-123".to_string(),
9440+
name: "test-provider".to_string(),
9441+
// resource_version and created_at_ms are 0
9442+
// labels is empty
9443+
..Default::default()
9444+
};
9445+
9446+
let provider = Provider {
9447+
metadata: Some(metadata),
9448+
r#type: "anthropic".to_string(),
9449+
credentials: std::collections::HashMap::new(),
9450+
config: std::collections::HashMap::new(),
9451+
credential_expires_at_ms: std::collections::HashMap::new(),
9452+
};
9453+
9454+
let json = super::provider_to_json(&provider);
9455+
9456+
assert!(
9457+
json.get("resource_version").is_none(),
9458+
"zero resource_version should be omitted"
9459+
);
9460+
assert!(
9461+
json.get("created_at").is_none(),
9462+
"zero created_at should be omitted"
9463+
);
9464+
assert!(
9465+
json.get("labels").is_none(),
9466+
"empty labels should be omitted"
9467+
);
9468+
}
9469+
9470+
#[test]
9471+
fn provider_to_json_includes_credential_expiration() {
9472+
let mut credential_expires_at_ms = std::collections::HashMap::new();
9473+
credential_expires_at_ms.insert("ACCESS_TOKEN".to_string(), 1_234_567_890);
9474+
9475+
let provider = Provider {
9476+
metadata: Some(ObjectMeta::default()),
9477+
r#type: "oauth".to_string(),
9478+
credentials: std::collections::HashMap::new(),
9479+
config: std::collections::HashMap::new(),
9480+
credential_expires_at_ms,
9481+
};
9482+
9483+
let json = super::provider_to_json(&provider);
9484+
9485+
assert_eq!(
9486+
json["credential_expires_at_ms"]["ACCESS_TOKEN"],
9487+
1_234_567_890
9488+
);
9489+
}
9490+
9491+
#[test]
9492+
fn provider_to_json_formats_created_at_as_human_readable() {
9493+
let metadata = ObjectMeta {
9494+
id: "prov-123".to_string(),
9495+
name: "test-provider".to_string(),
9496+
created_at_ms: 1_609_459_200_000, // 2021-01-01 00:00:00
9497+
..Default::default()
9498+
};
9499+
9500+
let provider = Provider {
9501+
metadata: Some(metadata),
9502+
r#type: "anthropic".to_string(),
9503+
credentials: std::collections::HashMap::new(),
9504+
config: std::collections::HashMap::new(),
9505+
credential_expires_at_ms: std::collections::HashMap::new(),
9506+
};
9507+
9508+
let json = super::provider_to_json(&provider);
9509+
9510+
// Should format as human-readable datetime, not raw milliseconds
9511+
assert_eq!(json["created_at"], "2021-01-01 00:00:00");
9512+
assert!(
9513+
json.get("created_at_ms").is_none(),
9514+
"raw milliseconds field should not exist"
9515+
);
9516+
}
92359517
}

0 commit comments

Comments
 (0)