From a632319d8af1cf1d681120158815b503659b61d2 Mon Sep 17 00:00:00 2001 From: Taylor Mutch Date: Fri, 17 Apr 2026 15:07:16 -0700 Subject: [PATCH] feat(cli): show active runtime policy when retrieving sandbox --- .agents/skills/openshell-cli/cli-reference.md | 12 +- architecture/security-policy.md | 13 +- crates/openshell-cli/src/main.rs | 8 +- crates/openshell-cli/src/run.rs | 180 ++++++++++++------ .../sandbox_name_fallback_integration.rs | 6 +- crates/openshell-server/src/grpc/policy.rs | 105 +++++++--- e2e/rust/tests/live_policy_update.rs | 23 +++ 7 files changed, 249 insertions(+), 98 deletions(-) diff --git a/.agents/skills/openshell-cli/cli-reference.md b/.agents/skills/openshell-cli/cli-reference.md index e344f20df..fe32cf0ab 100644 --- a/.agents/skills/openshell-cli/cli-reference.md +++ b/.agents/skills/openshell-cli/cli-reference.md @@ -181,7 +181,11 @@ Create a sandbox, wait for readiness, then connect or execute the trailing comma ### `openshell sandbox get ` -Show sandbox details (id, name, namespace, phase, policy). +Show sandbox details (id, name, namespace, phase) and the **active** policy from the gateway (`GetSandboxConfig`), not necessarily the creation-time spec. + +| Flag | Default | Description | +|------|---------|-------------| +| `--policy-only` | false | Print only the active policy as YAML (no sandbox header) | ### `openshell sandbox list` @@ -282,12 +286,12 @@ Exit codes with `--wait`: 0 = loaded, 1 = failed, 124 = timeout. ### `openshell policy get ` -Show current active policy for a sandbox. +Show the effective runtime policy for a sandbox (or a specific sandbox revision with `--rev`), including **Policy source** (`sandbox` vs `global` when a gateway-global override is active). | Flag | Default | Description | |------|---------|-------------| -| `--rev ` | 0 (latest) | Show a specific revision | -| `--full` | false | Print the full policy as YAML (round-trips with `--policy` input) | +| `--rev ` | 0 (latest) | `0` = effective policy (global wins when set). Non-zero = sandbox-scoped revision for audit. | +| `--full` | false | Print the full policy as YAML after the metadata summary | ### `openshell policy list ` diff --git a/architecture/security-policy.md b/architecture/security-policy.md index b8bda8f91..8d8a2d77f 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -233,6 +233,9 @@ openshell policy get --rev 3 # Print the full policy as YAML (round-trips with --policy input format) openshell policy get --full +# Print only the active policy YAML (from sandbox get; effective runtime policy) +openshell sandbox get --policy-only + # Combine: inspect a specific revision's full policy openshell policy get --rev 2 --full @@ -265,10 +268,16 @@ See [Gateway Settings Channel](gateway-settings.md#global-policy-lifecycle) for | Flag | Default | Description | |------|---------|-------------| -| `--rev N` | `0` (latest) | Retrieve a specific policy revision by version number instead of the latest. Maps to the `version` field of `GetSandboxPolicyStatusRequest` -- version `0` resolves to the latest revision server-side. | +| `--rev N` | `0` (latest) | With `N = 0`, resolves to the **effective** runtime policy: if a gateway-global policy is active, that global revision is returned; otherwise the latest sandbox-scoped revision. With `N > 0`, returns the **sandbox-scoped** revision `N` from history (audit); the CLI prints a note when a global policy is active because that revision is not the effective runtime policy. Maps to the `version` field of `GetSandboxPolicyStatusRequest`. | | `--full` | off | Print the complete policy as YAML after the metadata summary. The YAML output uses the same schema as the `--policy` input file, so it round-trips: you can save it to a file and pass it back to `nav policy set --policy`. | -When `--full` is specified, the server includes the deserialized `SandboxPolicy` protobuf in the `SandboxPolicyRevision.policy` field (see `crates/openshell-server/src/grpc.rs` -- `policy_record_to_revision()` with `include_policy: true`). The CLI converts this proto back to YAML via `policy_to_yaml()`, which uses a `BTreeMap` for `network_policies` to produce deterministic key ordering. See `crates/openshell-cli/src/run.rs` -- `policy_to_yaml()`, `policy_get()`. +Metadata includes **Policy source** (`sandbox` or `global`) matching `GetSandboxConfig` / runtime behavior, and **Global revision** when the effective policy is global. + +For **YAML only** of the active runtime policy (no revision metadata), use `openshell sandbox get --policy-only` (`GetSandboxConfig.policy`). + +When `--full` is specified, the server includes the deserialized `SandboxPolicy` protobuf in the `SandboxPolicyRevision.policy` field (see `crates/openshell-server/src/grpc.rs` -- `policy_record_to_revision()` with `include_policy: true`). The CLI converts this proto back to YAML via `serialize_sandbox_policy()`. See `crates/openshell-cli/src/run.rs` -- `sandbox_policy_get()`. + +The CLI loads **Policy source** and **Global revision** from `GetSandboxConfig` (same effective policy metadata as `openshell sandbox settings get`), while revision rows and YAML payloads come from `GetSandboxPolicyStatus`. See `crates/openshell-cli/src/main.rs` -- `PolicyCommands` enum, `crates/openshell-cli/src/run.rs` -- `policy_set()`, `policy_get()`, `policy_list()`. diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 292922411..0972b352e 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1206,6 +1206,10 @@ enum SandboxCommands { /// Sandbox name (defaults to last-used sandbox). #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] name: Option, + + /// Print only the active policy as YAML (effective runtime policy from the gateway). + #[arg(long)] + policy_only: bool, }, /// List sandboxes. @@ -2380,9 +2384,9 @@ async fn main() -> Result<()> { | SandboxCommands::Download { .. } => { unreachable!() } - SandboxCommands::Get { name } => { + SandboxCommands::Get { name, policy_only } => { let name = resolve_sandbox_name(name, &ctx.name)?; - run::sandbox_get(endpoint, &name, &tls).await?; + run::sandbox_get(endpoint, &name, policy_only, &tls).await?; } SandboxCommands::List { limit, diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index c41b53518..857d1f300 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -2733,7 +2733,16 @@ pub async fn sandbox_sync_command( } /// Fetch a sandbox by name. -pub async fn sandbox_get(server: &str, name: &str, tls: &TlsOptions) -> Result<()> { +/// +/// When `policy_only` is true, prints only the active (effective) policy YAML from +/// [`GetSandboxConfig`]. Otherwise prints sandbox metadata and the active policy +/// from the gateway (not necessarily the creation-time `spec.policy`). +pub async fn sandbox_get( + server: &str, + name: &str, + policy_only: bool, + tls: &TlsOptions, +) -> Result<()> { let mut client = grpc_client(server, tls).await?; let response = client @@ -2747,6 +2756,26 @@ pub async fn sandbox_get(server: &str, name: &str, tls: &TlsOptions) -> Result<( .sandbox .ok_or_else(|| miette::miette!("sandbox missing from response"))?; + let config = client + .get_sandbox_config(GetSandboxConfigRequest { + sandbox_id: sandbox.id.clone(), + }) + .await + .into_diagnostic()? + .into_inner(); + + if policy_only { + let Some(ref policy) = config.policy else { + return Err(miette::miette!( + "no active policy configured for this sandbox" + )); + }; + let yaml_str = openshell_policy::serialize_sandbox_policy(policy) + .wrap_err("failed to serialize policy to YAML")?; + print!("{yaml_str}"); + return Ok(()); + } + println!("{}", "Sandbox:".cyan().bold()); println!(); println!(" {} {}", "Id:".dimmed(), sandbox.id); @@ -2754,11 +2783,9 @@ pub async fn sandbox_get(server: &str, name: &str, tls: &TlsOptions) -> Result<( println!(" {} {}", "Namespace:".dimmed(), sandbox.namespace); println!(" {} {}", "Phase:".dimmed(), phase_name(sandbox.phase)); - if let Some(spec) = &sandbox.spec - && let Some(policy) = &spec.policy - { + if let Some(policy) = config.policy { println!(); - print_sandbox_policy(policy); + print_sandbox_policy(&policy); } Ok(()) @@ -4633,34 +4660,74 @@ pub async fn sandbox_policy_get( .into_diagnostic()?; let inner = status_resp.into_inner(); - if let Some(rev) = inner.revision { - let status = PolicyStatus::try_from(rev.status).unwrap_or(PolicyStatus::Unspecified); - println!("Version: {}", rev.version); - println!("Hash: {}", rev.policy_hash); - println!("Status: {status:?}"); - println!("Active: {}", inner.active_version); - if rev.created_at_ms > 0 { - println!("Created: {} ms", rev.created_at_ms); - } - if rev.loaded_at_ms > 0 { - println!("Loaded: {} ms", rev.loaded_at_ms); - } - if !rev.load_error.is_empty() { - println!("Error: {}", rev.load_error); - } - - if full { - if let Some(ref policy) = rev.policy { - println!("---"); - let yaml_str = openshell_policy::serialize_sandbox_policy(policy) - .wrap_err("failed to serialize policy to YAML")?; - print!("{yaml_str}"); - } else { - eprintln!("Policy payload not available for this version"); - } - } - } else { + let Some(rev) = inner.revision else { eprintln!("No policy history found for sandbox '{name}'"); + return Ok(()); + }; + + let sandbox_msg = client + .get_sandbox(GetSandboxRequest { + name: name.to_string(), + }) + .await + .into_diagnostic()? + .into_inner() + .sandbox + .ok_or_else(|| miette::miette!("sandbox not found"))?; + + let config = client + .get_sandbox_config(GetSandboxConfigRequest { + sandbox_id: sandbox_msg.id, + }) + .await + .into_diagnostic()? + .into_inner(); + + let global_effective = + config.policy_source == openshell_core::proto::PolicySource::Global as i32; + + let policy_source_label = if global_effective { + "global" + } else if config.policy_source == openshell_core::proto::PolicySource::Sandbox as i32 { + "sandbox" + } else { + "unspecified" + }; + println!("Policy source: {}", policy_source_label); + if global_effective && config.global_policy_version > 0 { + println!("Global revision: {}", config.global_policy_version); + } + if global_effective && version > 0 { + eprintln!( + "Note: a gateway-global policy is active; sandbox revision {} is not the effective runtime policy.", + version + ); + } + + let status = PolicyStatus::try_from(rev.status).unwrap_or(PolicyStatus::Unspecified); + println!("Version: {}", rev.version); + println!("Hash: {}", rev.policy_hash); + println!("Status: {status:?}"); + println!("Active: {}", inner.active_version); + if rev.created_at_ms > 0 { + println!("Created: {} ms", rev.created_at_ms); + } + if rev.loaded_at_ms > 0 { + println!("Loaded: {} ms", rev.loaded_at_ms); + } + if !rev.load_error.is_empty() { + println!("Error: {}", rev.load_error); + } + + if full { + if let Some(ref policy) = rev.policy { + println!("---"); + let yaml_str = openshell_policy::serialize_sandbox_policy(policy) + .wrap_err("failed to serialize policy to YAML")?; + print!("{yaml_str}"); + } else { + eprintln!("Policy payload not available for this version"); + } } Ok(()) @@ -4684,31 +4751,32 @@ pub async fn sandbox_policy_get_global( .into_diagnostic()?; let inner = status_resp.into_inner(); - if let Some(rev) = inner.revision { - let status = PolicyStatus::try_from(rev.status).unwrap_or(PolicyStatus::Unspecified); - println!("Scope: global"); - println!("Version: {}", rev.version); - println!("Hash: {}", rev.policy_hash); - println!("Status: {status:?}"); - if rev.created_at_ms > 0 { - println!("Created: {} ms", rev.created_at_ms); - } - if rev.loaded_at_ms > 0 { - println!("Loaded: {} ms", rev.loaded_at_ms); - } - - if full { - if let Some(ref policy) = rev.policy { - println!("---"); - let yaml_str = openshell_policy::serialize_sandbox_policy(policy) - .wrap_err("failed to serialize policy to YAML")?; - print!("{yaml_str}"); - } else { - eprintln!("Policy payload not available for this version"); - } - } - } else { + let Some(rev) = inner.revision else { eprintln!("No global policy history found"); + return Ok(()); + }; + + let status = PolicyStatus::try_from(rev.status).unwrap_or(PolicyStatus::Unspecified); + println!("Policy source: global"); + println!("Version: {}", rev.version); + println!("Hash: {}", rev.policy_hash); + println!("Status: {status:?}"); + if rev.created_at_ms > 0 { + println!("Created: {} ms", rev.created_at_ms); + } + if rev.loaded_at_ms > 0 { + println!("Loaded: {} ms", rev.loaded_at_ms); + } + + if full { + if let Some(ref policy) = rev.policy { + println!("---"); + let yaml_str = openshell_policy::serialize_sandbox_policy(policy) + .wrap_err("failed to serialize policy to YAML")?; + print!("{yaml_str}"); + } else { + eprintln!("Policy payload not available for this version"); + } } Ok(()) diff --git a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs index fbadec4c3..3b58404fc 100644 --- a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs +++ b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs @@ -432,7 +432,7 @@ async fn run_server() -> TestServer { async fn sandbox_get_sends_correct_name() { let ts = run_server().await; - run::sandbox_get(&ts.endpoint, "my-sandbox", &ts.tls) + run::sandbox_get(&ts.endpoint, "my-sandbox", false, &ts.tls) .await .expect("sandbox_get should succeed"); @@ -462,7 +462,7 @@ async fn sandbox_get_with_persisted_last_sandbox() { assert_eq!(resolved, "persisted-sb"); // Call sandbox_get with the resolved name. - run::sandbox_get(&ts.endpoint, &resolved, &ts.tls) + run::sandbox_get(&ts.endpoint, &resolved, false, &ts.tls) .await .expect("sandbox_get should succeed"); @@ -484,7 +484,7 @@ async fn explicit_name_takes_precedence_over_persisted() { // Persist one name, but supply a different one explicitly. save_last_sandbox("my-cluster", "old-sandbox").expect("save should succeed"); - run::sandbox_get(&ts.endpoint, "explicit-sandbox", &ts.tls) + run::sandbox_get(&ts.endpoint, "explicit-sandbox", false, &ts.tls) .await .expect("sandbox_get should succeed"); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 58d0c03cf..701e1ed16 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -592,45 +592,88 @@ pub(super) async fn handle_get_sandbox_policy_status( ) -> Result, Status> { let req = request.into_inner(); - let (policy_id, active_version) = if req.global { - (GLOBAL_POLICY_SANDBOX_ID.to_string(), 0_u32) - } else { - if req.name.is_empty() { - return Err(Status::invalid_argument("name is required")); + if req.global { + let policy_id = GLOBAL_POLICY_SANDBOX_ID.to_string(); + let record = if req.version == 0 { + state + .store + .get_latest_policy(&policy_id) + .await + .map_err(|e| Status::internal(format!("fetch policy failed: {e}")))? + } else { + state + .store + .get_policy_by_version(&policy_id, i64::from(req.version)) + .await + .map_err(|e| Status::internal(format!("fetch policy failed: {e}")))? + }; + let record = record.ok_or_else(|| Status::not_found("no global policy revision found"))?; + let gv = u32::try_from(record.version).unwrap_or(0); + return Ok(Response::new(GetSandboxPolicyStatusResponse { + revision: Some(policy_record_to_revision(&record, true)), + active_version: gv, + })); + } + + if req.name.is_empty() { + return Err(Status::invalid_argument("name is required")); + } + + let sandbox = state + .store + .get_message_by_name::(&req.name) + .await + .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? + .ok_or_else(|| Status::not_found("sandbox not found"))?; + + let sandbox_id = sandbox.id.clone(); + let sandbox_current = sandbox.current_policy_version; + + if req.version == 0 { + let global_settings = load_global_settings(state.store.as_ref()).await?; + let global_policy_active = decode_policy_from_global_settings(&global_settings)?.is_some(); + if global_policy_active { + let record = state + .store + .get_latest_policy(GLOBAL_POLICY_SANDBOX_ID) + .await + .map_err(|e| Status::internal(format!("fetch policy failed: {e}")))? + .ok_or_else(|| { + Status::internal( + "global policy is configured in settings but no global revision was found", + ) + })?; + let gv = u32::try_from(record.version).unwrap_or(0); + return Ok(Response::new(GetSandboxPolicyStatusResponse { + revision: Some(policy_record_to_revision(&record, true)), + active_version: gv, + })); } - let sandbox = state - .store - .get_message_by_name::(&req.name) - .await - .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? - .ok_or_else(|| Status::not_found("sandbox not found"))?; - (sandbox.id, sandbox.current_policy_version) - }; - let record = if req.version == 0 { - state + let record = state .store - .get_latest_policy(&policy_id) + .get_latest_policy(&sandbox_id) .await - .map_err(|e| Status::internal(format!("fetch policy failed: {e}")))? - } else { - state - .store - .get_policy_by_version(&policy_id, i64::from(req.version)) - .await - .map_err(|e| Status::internal(format!("fetch policy failed: {e}")))? - }; + .map_err(|e| Status::internal(format!("fetch policy failed: {e}")))?; + let record = + record.ok_or_else(|| Status::not_found("no policy revision found for this sandbox"))?; + return Ok(Response::new(GetSandboxPolicyStatusResponse { + revision: Some(policy_record_to_revision(&record, true)), + active_version: sandbox_current, + })); + } - let not_found_msg = if req.global { - "no global policy revision found" - } else { - "no policy revision found for this sandbox" - }; - let record = record.ok_or_else(|| Status::not_found(not_found_msg))?; + let record = state + .store + .get_policy_by_version(&sandbox_id, i64::from(req.version)) + .await + .map_err(|e| Status::internal(format!("fetch policy failed: {e}")))?; + let record = + record.ok_or_else(|| Status::not_found("no policy revision found for this sandbox"))?; Ok(Response::new(GetSandboxPolicyStatusResponse { revision: Some(policy_record_to_revision(&record, true)), - active_version, + active_version: sandbox_current, })) } diff --git a/e2e/rust/tests/live_policy_update.rs b/e2e/rust/tests/live_policy_update.rs index c60b29548..248f7be34 100644 --- a/e2e/rust/tests/live_policy_update.rs +++ b/e2e/rust/tests/live_policy_update.rs @@ -250,9 +250,32 @@ async fn live_policy_update_round_trip() { initial_version >= 1, "initial policy version should be >= 1, got {initial_version}" ); + assert!( + r.output.contains("Policy source: sandbox"), + "policy get should report sandbox policy source:\n{}", + r.output + ); let initial_hash = extract_hash(&r.output); + let r_yaml = run_cli(&["sandbox", "get", &guard.name, "--policy-only"]).await; + assert!( + r_yaml.success, + "sandbox get --policy-only should succeed (exit {:?}):\n{}", + r_yaml.exit_code, + r_yaml.output + ); + assert!( + !r_yaml.output.contains("Sandbox:"), + "sandbox get --policy-only should omit sandbox header:\n{}", + r_yaml.output + ); + assert!( + r_yaml.output.contains("version:"), + "sandbox get --policy-only should print policy YAML:\n{}", + r_yaml.output + ); + // --- Push same policy A again -> should be idempotent --- let r = run_cli(&[ "policy", "set", &guard.name, "--policy", &policy_a_path, "--wait", "--timeout", "120",