diff --git a/crates/navigator-cli/src/run.rs b/crates/navigator-cli/src/run.rs index 3d32ee81..c769bb61 100644 --- a/crates/navigator-cli/src/run.rs +++ b/crates/navigator-cli/src/run.rs @@ -35,7 +35,6 @@ use navigator_providers::{ }; use owo_colors::OwoColorize; use reqwest::StatusCode as ReqwestStatusCode; -use serde::Serialize; use std::collections::{HashMap, HashSet, VecDeque}; use std::io::{IsTerminal, Write}; use std::path::{Path, PathBuf}; @@ -1368,174 +1367,6 @@ pub async fn sandbox_get(server: &str, name: &str, tls: &TlsOptions) -> Result<( Ok(()) } -/// Serializable policy structure for YAML output. -#[derive(Serialize)] -struct PolicyYaml { - version: u32, - #[serde(skip_serializing_if = "Option::is_none")] - inference: Option, - #[serde(skip_serializing_if = "Option::is_none")] - filesystem: Option, - #[serde(skip_serializing_if = "Option::is_none")] - landlock: Option, - #[serde(skip_serializing_if = "Option::is_none")] - process: Option, - #[serde(skip_serializing_if = "std::collections::BTreeMap::is_empty")] - network_policies: std::collections::BTreeMap, -} - -#[derive(Serialize)] -struct InferenceYaml { - #[serde(skip_serializing_if = "Vec::is_empty")] - allowed_routes: Vec, -} - -#[derive(Serialize)] -struct FilesystemYaml { - include_workdir: bool, - #[serde(skip_serializing_if = "Vec::is_empty")] - read_only: Vec, - #[serde(skip_serializing_if = "Vec::is_empty")] - read_write: Vec, -} - -#[derive(Serialize)] -struct LandlockYaml { - compatibility: String, -} - -#[derive(Serialize)] -struct ProcessYaml { - #[serde(skip_serializing_if = "String::is_empty")] - run_as_user: String, - #[serde(skip_serializing_if = "String::is_empty")] - run_as_group: String, -} - -#[derive(Serialize)] -struct NetworkPolicyRuleYaml { - #[serde(skip_serializing_if = "Vec::is_empty")] - endpoints: Vec, - #[serde(skip_serializing_if = "Vec::is_empty")] - binaries: Vec, -} - -#[derive(Serialize)] -struct NetworkEndpointYaml { - host: String, - port: u32, - #[serde(skip_serializing_if = "String::is_empty")] - protocol: String, - #[serde(skip_serializing_if = "String::is_empty")] - tls: String, - #[serde(skip_serializing_if = "String::is_empty")] - enforcement: String, - #[serde(skip_serializing_if = "String::is_empty")] - access: String, - #[serde(skip_serializing_if = "Vec::is_empty")] - rules: Vec, -} - -#[derive(Serialize)] -struct L7RuleYaml { - allow: L7AllowYaml, -} - -#[derive(Serialize)] -struct L7AllowYaml { - #[serde(skip_serializing_if = "String::is_empty")] - method: String, - #[serde(skip_serializing_if = "String::is_empty")] - path: String, - #[serde(skip_serializing_if = "String::is_empty")] - command: String, -} - -#[derive(Serialize)] -struct NetworkBinaryYaml { - path: String, -} - -/// Convert proto policy to serializable YAML structure. -fn policy_to_yaml(policy: &SandboxPolicy) -> PolicyYaml { - let inference = policy.inference.as_ref().map(|inf| InferenceYaml { - allowed_routes: inf.allowed_routes.clone(), - }); - - let filesystem = policy.filesystem.as_ref().map(|fs| FilesystemYaml { - include_workdir: fs.include_workdir, - read_only: fs.read_only.clone(), - read_write: fs.read_write.clone(), - }); - - let landlock = policy.landlock.as_ref().map(|ll| LandlockYaml { - compatibility: ll.compatibility.clone(), - }); - - let process = policy.process.as_ref().and_then(|p| { - if p.run_as_user.is_empty() && p.run_as_group.is_empty() { - None - } else { - Some(ProcessYaml { - run_as_user: p.run_as_user.clone(), - run_as_group: p.run_as_group.clone(), - }) - } - }); - - let network_policies = policy - .network_policies - .iter() - .map(|(key, rule)| { - let yaml_rule = NetworkPolicyRuleYaml { - endpoints: rule - .endpoints - .iter() - .map(|e| NetworkEndpointYaml { - host: e.host.clone(), - port: e.port, - protocol: e.protocol.clone(), - tls: e.tls.clone(), - enforcement: e.enforcement.clone(), - access: e.access.clone(), - rules: e - .rules - .iter() - .map(|r| { - let a = r.allow.clone().unwrap_or_default(); - L7RuleYaml { - allow: L7AllowYaml { - method: a.method, - path: a.path, - command: a.command, - }, - } - }) - .collect(), - }) - .collect(), - binaries: rule - .binaries - .iter() - .map(|b| NetworkBinaryYaml { - path: b.path.clone(), - }) - .collect(), - }; - (key.clone(), yaml_rule) - }) - .collect(); - - PolicyYaml { - version: policy.version, - inference, - filesystem, - landlock, - process, - network_policies, - } -} - /// Print a single YAML line with dimmed keys and regular values. fn print_yaml_line(line: &str) { // Find leading whitespace @@ -1581,8 +1412,7 @@ fn print_yaml_line(line: &str) { fn print_sandbox_policy(policy: &SandboxPolicy) { println!("{}", "Policy:".cyan().bold()); println!(); - let policy_yaml = policy_to_yaml(policy); - if let Ok(yaml_str) = serde_yaml::to_string(&policy_yaml) { + if let Ok(yaml_str) = navigator_policy::serialize_sandbox_policy(policy) { // Indent the YAML output and skip the initial "---" line for line in yaml_str.lines() { if line == "---" { @@ -2899,11 +2729,9 @@ pub async fn sandbox_policy_get( } if full { - if let Some(policy) = rev.policy { + if let Some(ref policy) = rev.policy { println!("---"); - let yaml_repr = policy_to_yaml(&policy); - let yaml_str = serde_yaml::to_string(&yaml_repr) - .into_diagnostic() + let yaml_str = navigator_policy::serialize_sandbox_policy(policy) .wrap_err("failed to serialize policy to YAML")?; print!("{yaml_str}"); } else { diff --git a/crates/navigator-policy/src/lib.rs b/crates/navigator-policy/src/lib.rs index 53d6b9de..cec8a1ec 100644 --- a/crates/navigator-policy/src/lib.rs +++ b/crates/navigator-policy/src/lib.rs @@ -3,129 +3,148 @@ //! Shared sandbox policy parsing and defaults for NemoClaw. //! -//! Provides YAML→proto conversion for sandbox policies, with a built-in -//! default policy embedded from `dev-sandbox-policy.yaml`. +//! Provides bidirectional YAML↔proto conversion for sandbox policies, with a +//! built-in default policy embedded from `dev-sandbox-policy.yaml`. +//! +//! The serde types here are the **single canonical representation** of the YAML +//! policy schema. Both parsing (YAML→proto) and serialization (proto→YAML) use +//! these types, ensuring round-trip fidelity. -use std::collections::HashMap; +use std::collections::BTreeMap; use miette::{IntoDiagnostic, Result, WrapErr}; use navigator_core::proto::{ - FilesystemPolicy, L7Allow, L7Rule, LandlockPolicy, NetworkBinary, NetworkEndpoint, - NetworkPolicyRule, ProcessPolicy, SandboxPolicy, + self, FilesystemPolicy, InferenceApiPattern, L7Allow, L7Rule, LandlockPolicy, NetworkBinary, + NetworkEndpoint, NetworkPolicyRule, ProcessPolicy, SandboxPolicy, }; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; /// Built-in default sandbox policy YAML (embedded at compile time). const DEFAULT_SANDBOX_POLICY_YAML: &str = include_str!("../../../dev-sandbox-policy.yaml"); // --------------------------------------------------------------------------- -// YAML serde types +// YAML serde types (canonical — used for both parsing and serialization) // --------------------------------------------------------------------------- -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct PolicyFile { version: u32, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] inference: Option, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] filesystem_policy: Option, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] landlock: Option, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] process: Option, - #[serde(default)] - network_policies: HashMap, + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + network_policies: BTreeMap, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct FilesystemDef { #[serde(default)] include_workdir: bool, - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] read_only: Vec, - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] read_write: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct LandlockDef { - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] compatibility: String, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct ProcessDef { - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] run_as_user: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] run_as_group: String, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct InferenceDef { - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] allowed_routes: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + api_patterns: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +struct InferenceApiPatternDef { + #[serde(default, skip_serializing_if = "String::is_empty")] + method: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + path_glob: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + protocol: String, + #[serde(default, skip_serializing_if = "String::is_empty")] + kind: String, +} + +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct NetworkPolicyRuleDef { - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] name: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] endpoints: Vec, - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] binaries: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct NetworkEndpointDef { - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] host: String, port: u32, - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] protocol: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] tls: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] enforcement: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] access: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] rules: Vec, - #[serde(default)] + #[serde(default, skip_serializing_if = "Vec::is_empty")] allowed_ips: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct L7RuleDef { allow: L7AllowDef, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct L7AllowDef { - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] method: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] path: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] command: String, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] #[serde(deny_unknown_fields)] struct NetworkBinaryDef { path: String, /// Deprecated: ignored. Kept for backward compat with existing YAML files. - #[serde(default)] + #[serde(default, skip_serializing)] #[allow(dead_code)] harness: bool, } @@ -134,7 +153,7 @@ struct NetworkBinaryDef { // YAML → proto conversion // --------------------------------------------------------------------------- -fn convert_policy(raw: PolicyFile) -> SandboxPolicy { +fn to_proto(raw: PolicyFile) -> SandboxPolicy { let network_policies = raw .network_policies .into_iter() @@ -197,12 +216,115 @@ fn convert_policy(raw: PolicyFile) -> SandboxPolicy { run_as_group: p.run_as_group, }), network_policies, - inference: raw - .inference - .map(|inf| navigator_core::proto::InferencePolicy { - allowed_routes: inf.allowed_routes, - ..Default::default() - }), + inference: raw.inference.map(|inf| proto::InferencePolicy { + allowed_routes: inf.allowed_routes, + api_patterns: inf + .api_patterns + .into_iter() + .map(|p| InferenceApiPattern { + method: p.method, + path_glob: p.path_glob, + protocol: p.protocol, + kind: p.kind, + }) + .collect(), + }), + } +} + +// --------------------------------------------------------------------------- +// Proto → YAML conversion +// --------------------------------------------------------------------------- + +fn from_proto(policy: &SandboxPolicy) -> PolicyFile { + let inference = policy.inference.as_ref().map(|inf| InferenceDef { + allowed_routes: inf.allowed_routes.clone(), + api_patterns: inf + .api_patterns + .iter() + .map(|p| InferenceApiPatternDef { + method: p.method.clone(), + path_glob: p.path_glob.clone(), + protocol: p.protocol.clone(), + kind: p.kind.clone(), + }) + .collect(), + }); + + let filesystem_policy = policy.filesystem.as_ref().map(|fs| FilesystemDef { + include_workdir: fs.include_workdir, + read_only: fs.read_only.clone(), + read_write: fs.read_write.clone(), + }); + + let landlock = policy.landlock.as_ref().map(|ll| LandlockDef { + compatibility: ll.compatibility.clone(), + }); + + let process = policy.process.as_ref().and_then(|p| { + if p.run_as_user.is_empty() && p.run_as_group.is_empty() { + None + } else { + Some(ProcessDef { + run_as_user: p.run_as_user.clone(), + run_as_group: p.run_as_group.clone(), + }) + } + }); + + let network_policies = policy + .network_policies + .iter() + .map(|(key, rule)| { + let yaml_rule = NetworkPolicyRuleDef { + name: rule.name.clone(), + endpoints: rule + .endpoints + .iter() + .map(|e| NetworkEndpointDef { + host: e.host.clone(), + port: e.port, + protocol: e.protocol.clone(), + tls: e.tls.clone(), + enforcement: e.enforcement.clone(), + access: e.access.clone(), + rules: e + .rules + .iter() + .map(|r| { + let a = r.allow.clone().unwrap_or_default(); + L7RuleDef { + allow: L7AllowDef { + method: a.method, + path: a.path, + command: a.command, + }, + } + }) + .collect(), + allowed_ips: e.allowed_ips.clone(), + }) + .collect(), + binaries: rule + .binaries + .iter() + .map(|b| NetworkBinaryDef { + path: b.path.clone(), + harness: false, + }) + .collect(), + }; + (key.clone(), yaml_rule) + }) + .collect(); + + PolicyFile { + version: policy.version, + inference, + filesystem_policy, + landlock, + process, + network_policies, } } @@ -215,7 +337,19 @@ pub fn parse_sandbox_policy(yaml: &str) -> Result { let raw: PolicyFile = serde_yaml::from_str(yaml) .into_diagnostic() .wrap_err("failed to parse sandbox policy YAML")?; - Ok(convert_policy(raw)) + Ok(to_proto(raw)) +} + +/// Serialize a proto sandbox policy to a YAML string. +/// +/// This is the inverse of [`parse_sandbox_policy`] — the output uses the +/// canonical YAML field names (e.g. `filesystem_policy`, not `filesystem`) +/// and is round-trippable through `parse_sandbox_policy`. +pub fn serialize_sandbox_policy(policy: &SandboxPolicy) -> Result { + let yaml_repr = from_proto(policy); + serde_yaml::to_string(&yaml_repr) + .into_diagnostic() + .wrap_err("failed to serialize policy to YAML") } /// Load a sandbox policy with the standard resolution order: @@ -257,3 +391,140 @@ pub fn clear_process_identity(policy: &mut SandboxPolicy) { process.run_as_group = String::new(); } } + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + /// Round-trip: parse the built-in default policy YAML → proto → YAML → proto + /// and verify the two proto representations are identical. + #[test] + fn round_trip_default_policy() { + let proto1 = parse_sandbox_policy(DEFAULT_SANDBOX_POLICY_YAML) + .expect("failed to parse default policy"); + + let yaml_str = + serialize_sandbox_policy(&proto1).expect("failed to serialize policy to YAML"); + + let proto2 = parse_sandbox_policy(&yaml_str).expect("failed to re-parse serialized policy"); + + assert_eq!(proto1.version, proto2.version); + assert_eq!(proto1.filesystem, proto2.filesystem); + assert_eq!(proto1.landlock, proto2.landlock); + assert_eq!(proto1.process, proto2.process); + assert_eq!(proto1.inference, proto2.inference); + + // Compare network policies (proto HashMap ordering may differ, so + // compare key-by-key). + assert_eq!( + proto1.network_policies.len(), + proto2.network_policies.len(), + "network policy count mismatch" + ); + for (key, rule1) in &proto1.network_policies { + let rule2 = proto2 + .network_policies + .get(key) + .unwrap_or_else(|| panic!("missing network policy key: {key}")); + assert_eq!(rule1, rule2, "network policy mismatch for key: {key}"); + } + } + + /// Verify that the serialized YAML uses `filesystem_policy` (not + /// `filesystem`) so it can be fed back to `parse_sandbox_policy`. + #[test] + fn serialized_yaml_uses_filesystem_policy_key() { + let proto = default_sandbox_policy(); + let yaml = serialize_sandbox_policy(&proto).expect("serialize failed"); + assert!( + yaml.contains("filesystem_policy:"), + "expected `filesystem_policy:` in YAML output, got:\n{yaml}" + ); + assert!( + !yaml.contains("\nfilesystem:"), + "unexpected bare `filesystem:` key in YAML output" + ); + } + + /// Verify that `allowed_ips` survives the round-trip. + #[test] + fn round_trip_preserves_allowed_ips() { + let yaml = r#" +version: 1 +network_policies: + internal: + name: internal + endpoints: + - host: db.internal.corp + port: 5432 + allowed_ips: + - "10.0.5.0/24" + - "10.0.6.0/24" + binaries: + - path: /usr/bin/curl +"#; + let proto1 = parse_sandbox_policy(yaml).expect("parse failed"); + let yaml_out = serialize_sandbox_policy(&proto1).expect("serialize failed"); + let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed"); + + let ep1 = &proto1.network_policies["internal"].endpoints[0]; + let ep2 = &proto2.network_policies["internal"].endpoints[0]; + assert_eq!(ep1.allowed_ips, ep2.allowed_ips); + assert_eq!(ep1.allowed_ips, vec!["10.0.5.0/24", "10.0.6.0/24"]); + } + + /// Verify that the network policy `name` field survives the round-trip. + #[test] + fn round_trip_preserves_policy_name() { + let yaml = r#" +version: 1 +network_policies: + my_api: + name: my-custom-api-name + endpoints: + - host: api.example.com + port: 443 + binaries: + - path: /usr/bin/curl +"#; + let proto1 = parse_sandbox_policy(yaml).expect("parse failed"); + assert_eq!(proto1.network_policies["my_api"].name, "my-custom-api-name"); + + let yaml_out = serialize_sandbox_policy(&proto1).expect("serialize failed"); + let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed"); + assert_eq!(proto2.network_policies["my_api"].name, "my-custom-api-name"); + } + + /// Verify that `api_patterns` on inference survives the round-trip. + #[test] + fn round_trip_preserves_api_patterns() { + let yaml = r#" +version: 1 +inference: + allowed_routes: + - local + api_patterns: + - method: POST + path_glob: "/v1/chat/completions" + protocol: openai_chat_completions + kind: chat_completion +"#; + let proto1 = parse_sandbox_policy(yaml).expect("parse failed"); + assert_eq!(proto1.inference.as_ref().unwrap().api_patterns.len(), 1); + + let yaml_out = serialize_sandbox_policy(&proto1).expect("serialize failed"); + let proto2 = parse_sandbox_policy(&yaml_out).expect("re-parse failed"); + + let patterns1 = &proto1.inference.as_ref().unwrap().api_patterns; + let patterns2 = &proto2.inference.as_ref().unwrap().api_patterns; + assert_eq!(patterns1.len(), patterns2.len()); + assert_eq!(patterns1[0].method, patterns2[0].method); + assert_eq!(patterns1[0].path_glob, patterns2[0].path_glob); + assert_eq!(patterns1[0].protocol, patterns2[0].protocol); + assert_eq!(patterns1[0].kind, patterns2[0].kind); + } +} diff --git a/e2e/python/test_sandbox_policy.py b/e2e/python/test_sandbox_policy.py index c4b10e96..11835f21 100644 --- a/e2e/python/test_sandbox_policy.py +++ b/e2e/python/test_sandbox_policy.py @@ -387,7 +387,7 @@ def test_l4_cross_policy_denied( def test_l4_non_connect_method_rejected( sandbox: Callable[..., Sandbox], ) -> None: - """L4-6: Non-CONNECT HTTP method -> rejected with 405.""" + """L4-6: Non-CONNECT HTTP method -> rejected with 403.""" def send_get_to_proxy() -> str: import socket @@ -416,7 +416,7 @@ def send_get_to_proxy() -> str: with sandbox(spec=spec, delete_on_exit=True) as sb: result = sb.exec_python(send_get_to_proxy) assert result.exit_code == 0, result.stderr - assert "405" in result.stdout + assert "403" in result.stdout def test_l4_log_fields(