From 141d190cfdc18fcf1c473c54f7eb231a846ca0b5 Mon Sep 17 00:00:00 2001 From: mjamiv Date: Fri, 15 May 2026 20:42:25 +0000 Subject: [PATCH 1/2] fix(cli): add json output for policy get --- Cargo.lock | 1 + crates/openshell-cli/src/main.rs | 67 +++++++++++++++++++++- crates/openshell-cli/src/run.rs | 89 ++++++++++++++++++++++++++++++ crates/openshell-policy/Cargo.toml | 1 + crates/openshell-policy/src/lib.rs | 43 +++++++++++++++ 5 files changed, 199 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cba681774..1588558c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3581,6 +3581,7 @@ dependencies = [ "miette", "openshell-core", "serde", + "serde_json", "serde_yml", ] diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index ca242be32..1ebf032db 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -663,6 +663,21 @@ impl ProviderProfileOutput { } } +#[derive(Clone, Debug, ValueEnum)] +enum PolicyGetOutput { + Table, + Json, +} + +impl PolicyGetOutput { + fn as_str(&self) -> &'static str { + match self { + Self::Table => "table", + Self::Json => "json", + } + } +} + #[derive(Clone, Debug, ValueEnum)] enum CliEditor { Vscode, @@ -1491,6 +1506,10 @@ enum PolicyCommands { #[arg(long)] full: bool, + /// Output format. + #[arg(short = 'o', long = "output", value_enum, default_value_t = PolicyGetOutput::Table)] + output: PolicyGetOutput, + /// Show the global policy revision. #[arg(long)] global: bool, @@ -2167,13 +2186,29 @@ async fn main() -> Result<()> { name, rev, full, + output, global, } => { if global { - run::sandbox_policy_get_global(&ctx.endpoint, rev, full, &tls).await?; + run::sandbox_policy_get_global( + &ctx.endpoint, + rev, + full, + output.as_str(), + &tls, + ) + .await?; } else { let name = resolve_sandbox_name(name, &ctx.name)?; - run::sandbox_policy_get(&ctx.endpoint, &name, rev, full, &tls).await?; + run::sandbox_policy_get( + &ctx.endpoint, + &name, + rev, + full, + output.as_str(), + &tls, + ) + .await?; } } PolicyCommands::List { @@ -3557,6 +3592,34 @@ mod tests { } } + #[test] + fn policy_get_json_output_parses() { + let cli = Cli::try_parse_from([ + "openshell", + "policy", + "get", + "my-sandbox", + "--full", + "-o", + "json", + ]) + .expect("policy get -o json should parse"); + + match cli.command { + Some(Commands::Policy { + command: + Some(PolicyCommands::Get { + name, full, output, .. + }), + }) => { + assert_eq!(name.as_deref(), Some("my-sandbox")); + assert!(full); + assert!(matches!(output, PolicyGetOutput::Json)); + } + other => panic!("expected policy get command, got: {other:?}"), + } + } + #[test] fn policy_delete_global_parses() { let cli = Cli::try_parse_from(["openshell", "policy", "delete", "--global", "--yes"]) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 198cb4b0a..1695521b4 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5653,6 +5653,7 @@ pub async fn sandbox_policy_get( name: &str, version: u32, full: bool, + output: &str, tls: &TlsOptions, ) -> Result<()> { let mut client = grpc_client(server, tls).await?; @@ -5669,6 +5670,23 @@ pub async fn sandbox_policy_get( let inner = status_resp.into_inner(); if let Some(rev) = inner.revision { let status = PolicyStatus::try_from(rev.status).unwrap_or(PolicyStatus::Unspecified); + match output { + "json" => { + let obj = policy_revision_to_json( + "sandbox", + Some(name), + Some(inner.active_version), + &rev, + status, + full, + )?; + println!("{}", serde_json::to_string_pretty(&obj).into_diagnostic()?); + return Ok(()); + } + "table" => {} + _ => return Err(miette!("unsupported output format: {output}")), + } + println!("Version: {}", rev.version); println!("Hash: {}", rev.policy_hash); println!("Status: {status:?}"); @@ -5704,6 +5722,7 @@ pub async fn sandbox_policy_get_global( server: &str, version: u32, full: bool, + output: &str, tls: &TlsOptions, ) -> Result<()> { let mut client = grpc_client(server, tls).await?; @@ -5720,6 +5739,16 @@ pub async fn sandbox_policy_get_global( let inner = status_resp.into_inner(); if let Some(rev) = inner.revision { let status = PolicyStatus::try_from(rev.status).unwrap_or(PolicyStatus::Unspecified); + match output { + "json" => { + let obj = policy_revision_to_json("global", None, None, &rev, status, full)?; + println!("{}", serde_json::to_string_pretty(&obj).into_diagnostic()?); + return Ok(()); + } + "table" => {} + _ => return Err(miette!("unsupported output format: {output}")), + } + println!("Scope: global"); println!("Version: {}", rev.version); println!("Hash: {}", rev.policy_hash); @@ -5748,6 +5777,66 @@ pub async fn sandbox_policy_get_global( Ok(()) } +fn policy_status_json_name(status: PolicyStatus) -> &'static str { + match status { + PolicyStatus::Unspecified => "unspecified", + PolicyStatus::Pending => "pending", + PolicyStatus::Loaded => "loaded", + PolicyStatus::Failed => "failed", + PolicyStatus::Superseded => "superseded", + } +} + +fn policy_revision_to_json( + scope: &str, + sandbox: Option<&str>, + active_version: Option, + rev: &openshell_core::proto::SandboxPolicyRevision, + status: PolicyStatus, + full: bool, +) -> Result { + let mut obj = serde_json::Map::new(); + obj.insert("scope".to_string(), serde_json::json!(scope)); + if let Some(sandbox) = sandbox { + obj.insert("sandbox".to_string(), serde_json::json!(sandbox)); + } + obj.insert("version".to_string(), serde_json::json!(rev.version)); + obj.insert("hash".to_string(), serde_json::json!(rev.policy_hash)); + obj.insert( + "status".to_string(), + serde_json::json!(policy_status_json_name(status)), + ); + if let Some(active_version) = active_version { + obj.insert( + "active_version".to_string(), + serde_json::json!(active_version), + ); + } + if rev.created_at_ms > 0 { + obj.insert( + "created_at_ms".to_string(), + serde_json::json!(rev.created_at_ms), + ); + } + if rev.loaded_at_ms > 0 { + obj.insert( + "loaded_at_ms".to_string(), + serde_json::json!(rev.loaded_at_ms), + ); + } + if !rev.load_error.is_empty() { + obj.insert("load_error".to_string(), serde_json::json!(rev.load_error)); + } + if full { + let policy = match rev.policy.as_ref() { + Some(policy) => openshell_policy::sandbox_policy_to_json_value(policy)?, + None => serde_json::Value::Null, + }; + obj.insert("policy".to_string(), policy); + } + Ok(serde_json::Value::Object(obj)) +} + pub async fn sandbox_policy_list( server: &str, name: &str, diff --git a/crates/openshell-policy/Cargo.toml b/crates/openshell-policy/Cargo.toml index f26136c6b..8936b85be 100644 --- a/crates/openshell-policy/Cargo.toml +++ b/crates/openshell-policy/Cargo.toml @@ -13,6 +13,7 @@ repository.workspace = true [dependencies] openshell-core = { path = "../openshell-core" } serde = { workspace = true } +serde_json = { workspace = true } serde_yml = { workspace = true } miette = { workspace = true } diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 632170c0b..8dbaf077c 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -558,6 +558,25 @@ pub fn serialize_sandbox_policy(policy: &SandboxPolicy) -> Result { .wrap_err("failed to serialize policy to YAML") } +/// Convert a proto sandbox policy into the canonical policy JSON representation. +/// +/// The shape mirrors the YAML schema used by [`serialize_sandbox_policy`], so +/// automation can use the same documented field names in either format. +pub fn sandbox_policy_to_json_value(policy: &SandboxPolicy) -> Result { + let json_repr = from_proto(policy); + serde_json::to_value(&json_repr) + .into_diagnostic() + .wrap_err("failed to serialize policy to JSON") +} + +/// Serialize a proto sandbox policy to a pretty-printed JSON string. +pub fn serialize_sandbox_policy_json(policy: &SandboxPolicy) -> Result { + let json_repr = sandbox_policy_to_json_value(policy)?; + serde_json::to_string_pretty(&json_repr) + .into_diagnostic() + .wrap_err("failed to serialize policy to JSON") +} + /// Load a sandbox policy from an explicit source. /// /// Resolution order: @@ -881,6 +900,30 @@ mod tests { ); } + /// Verify that JSON serialization uses the same canonical schema keys as YAML. + #[test] + fn serialized_json_uses_policy_schema_keys() { + let proto = parse_sandbox_policy( + r" +version: 1 +network_policies: + github: + endpoints: + - host: api.github.com + port: 443 + protocol: https + binaries: + - path: /usr/bin/curl +", + ) + .expect("parse failed"); + let json = sandbox_policy_to_json_value(&proto).expect("serialize failed"); + + assert_eq!(json["version"], serde_json::json!(1)); + assert!(json.get("filesystem").is_none()); + assert!(json.get("network_policies").is_some()); + } + /// Verify that `allowed_ips` survives the round-trip. #[test] fn round_trip_preserves_allowed_ips() { From 503dc1af1153470cf1415368d6b372aee10f10ba Mon Sep 17 00:00:00 2001 From: mjamiv Date: Thu, 21 May 2026 07:11:10 +0000 Subject: [PATCH 2/2] test(cli): cover policy get full json output --- crates/openshell-cli/src/main.rs | 2 +- crates/openshell-cli/src/run.rs | 56 ++++++++--- .../sandbox_name_fallback_integration.rs | 97 +++++++++++++++++-- 3 files changed, 133 insertions(+), 22 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 1ebf032db..bea4b96b2 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1502,7 +1502,7 @@ enum PolicyCommands { #[arg(long = "rev", default_value_t = 0)] rev: u32, - /// Print the full policy as YAML. + /// Include the full policy payload. #[arg(long)] full: bool, diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 1695521b4..730c38b88 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5655,6 +5655,32 @@ pub async fn sandbox_policy_get( full: bool, output: &str, tls: &TlsOptions, +) -> Result<()> { + let mut stdout = std::io::stdout().lock(); + let mut stderr = std::io::stderr().lock(); + sandbox_policy_get_to_writer( + server, + name, + version, + full, + output, + tls, + &mut stdout, + &mut stderr, + ) + .await +} + +#[doc(hidden)] +pub async fn sandbox_policy_get_to_writer( + server: &str, + name: &str, + version: u32, + full: bool, + output: &str, + tls: &TlsOptions, + stdout: &mut W, + stderr: &mut E, ) -> Result<()> { let mut client = grpc_client(server, tls).await?; @@ -5680,39 +5706,45 @@ pub async fn sandbox_policy_get( status, full, )?; - println!("{}", serde_json::to_string_pretty(&obj).into_diagnostic()?); + writeln!( + stdout, + "{}", + serde_json::to_string_pretty(&obj).into_diagnostic()? + ) + .into_diagnostic()?; return Ok(()); } "table" => {} _ => return Err(miette!("unsupported output format: {output}")), } - println!("Version: {}", rev.version); - println!("Hash: {}", rev.policy_hash); - println!("Status: {status:?}"); - println!("Active: {}", inner.active_version); + writeln!(stdout, "Version: {}", rev.version).into_diagnostic()?; + writeln!(stdout, "Hash: {}", rev.policy_hash).into_diagnostic()?; + writeln!(stdout, "Status: {status:?}").into_diagnostic()?; + writeln!(stdout, "Active: {}", inner.active_version).into_diagnostic()?; if rev.created_at_ms > 0 { - println!("Created: {} ms", rev.created_at_ms); + writeln!(stdout, "Created: {} ms", rev.created_at_ms).into_diagnostic()?; } if rev.loaded_at_ms > 0 { - println!("Loaded: {} ms", rev.loaded_at_ms); + writeln!(stdout, "Loaded: {} ms", rev.loaded_at_ms).into_diagnostic()?; } if !rev.load_error.is_empty() { - println!("Error: {}", rev.load_error); + writeln!(stdout, "Error: {}", rev.load_error).into_diagnostic()?; } if full { if let Some(ref policy) = rev.policy { - println!("---"); + writeln!(stdout, "---").into_diagnostic()?; let yaml_str = openshell_policy::serialize_sandbox_policy(policy) .wrap_err("failed to serialize policy to YAML")?; - print!("{yaml_str}"); + write!(stdout, "{yaml_str}").into_diagnostic()?; } else { - eprintln!("Policy payload not available for this version"); + writeln!(stderr, "Policy payload not available for this version") + .into_diagnostic()?; } } } else { - eprintln!("No policy history found for sandbox '{name}'"); + writeln!(stderr, "No policy history found for sandbox '{name}'").into_diagnostic()?; } Ok(()) diff --git a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs index 531599dcf..5ddda9a84 100644 --- a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs +++ b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs @@ -17,12 +17,13 @@ use openshell_core::proto::{ DetachSandboxProviderRequest, DetachSandboxProviderResponse, ExecSandboxEvent, ExecSandboxInput, ExecSandboxRequest, GatewayMessage, GetGatewayConfigRequest, GetGatewayConfigResponse, GetProviderRequest, GetSandboxConfigRequest, - GetSandboxConfigResponse, GetSandboxProviderEnvironmentRequest, - GetSandboxProviderEnvironmentResponse, GetSandboxRequest, HealthRequest, HealthResponse, - ListProvidersRequest, ListProvidersResponse, ListSandboxProvidersRequest, - ListSandboxProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, ProviderResponse, - Sandbox, SandboxPolicy, SandboxResponse, SandboxStreamEvent, ServiceStatus, SupervisorMessage, - UpdateProviderRequest, WatchSandboxRequest, + GetSandboxConfigResponse, GetSandboxPolicyStatusRequest, GetSandboxPolicyStatusResponse, + GetSandboxProviderEnvironmentRequest, GetSandboxProviderEnvironmentResponse, GetSandboxRequest, + HealthRequest, HealthResponse, ListProvidersRequest, ListProvidersResponse, + ListSandboxProvidersRequest, ListSandboxProvidersResponse, ListSandboxesRequest, + ListSandboxesResponse, NetworkEndpoint, NetworkPolicyRule, PolicyStatus, ProviderResponse, + Sandbox, SandboxPolicy, SandboxPolicyRevision, SandboxResponse, SandboxStreamEvent, + ServiceStatus, SupervisorMessage, UpdateProviderRequest, WatchSandboxRequest, }; use std::sync::Arc; use tempfile::TempDir; @@ -313,9 +314,47 @@ impl OpenShell for TestOpenShell { async fn get_sandbox_policy_status( &self, - _request: tonic::Request, - ) -> Result, Status> { - Err(Status::unimplemented("not implemented in test")) + request: tonic::Request, + ) -> Result, Status> { + let req = request.into_inner(); + assert_eq!(req.name, "my-sandbox"); + assert_eq!(req.version, 0); + assert!(!req.global); + + let policy = SandboxPolicy { + version: 7, + network_policies: [( + "api".to_string(), + NetworkPolicyRule { + name: "api".to_string(), + endpoints: vec![NetworkEndpoint { + host: "api.example.com".to_string(), + port: 443, + protocol: "rest".to_string(), + enforcement: "enforce".to_string(), + access: "read-only".to_string(), + ..Default::default() + }], + ..Default::default() + }, + )] + .into_iter() + .collect(), + ..Default::default() + }; + + Ok(Response::new(GetSandboxPolicyStatusResponse { + revision: Some(SandboxPolicyRevision { + version: 7, + policy_hash: "sha256:test-policy".to_string(), + status: PolicyStatus::Loaded.into(), + created_at_ms: 1_700_000_000_000, + loaded_at_ms: 1_700_000_000_500, + policy: Some(policy), + ..Default::default() + }), + active_version: 7, + })) } async fn list_sandbox_policies( @@ -558,6 +597,46 @@ async fn sandbox_get_with_persisted_last_sandbox() { ); } +#[tokio::test] +async fn policy_get_full_json_cli_prints_policy_payload() { + let ts = run_server().await; + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + + run::sandbox_policy_get_to_writer( + &ts.endpoint, + "my-sandbox", + 0, + true, + "json", + &ts.tls, + &mut stdout, + &mut stderr, + ) + .await + .expect("policy get should succeed"); + + assert!( + stderr.is_empty(), + "policy get should not print stderr: {}", + String::from_utf8_lossy(&stderr) + ); + + let json: serde_json::Value = + serde_json::from_slice(&stdout).expect("stdout should be valid JSON"); + assert_eq!(json["scope"], "sandbox"); + assert_eq!(json["sandbox"], "my-sandbox"); + assert_eq!(json["version"], 7); + assert_eq!(json["active_version"], 7); + assert_eq!(json["hash"], "sha256:test-policy"); + assert_eq!(json["status"], "loaded"); + assert_eq!(json["policy"]["network_policies"]["api"]["name"], "api"); + assert_eq!( + json["policy"]["network_policies"]["api"]["endpoints"][0]["host"], + "api.example.com" + ); +} + /// Verify that an explicit name takes precedence over the persisted one. #[tokio::test] async fn explicit_name_takes_precedence_over_persisted() {