From b49c0400c66165e075ff989b1a2bd9748ee6a14b Mon Sep 17 00:00:00 2001 From: Shiju Date: Sat, 7 Nov 2026 14:31:42 +0530 Subject: [PATCH 1/2] feat(policy): lint authored policy files offline Validate authored files before gateway selection or authentication. Share creation safety checks and report file-only violations together while deferring checks that require live gateway state. Closes #4035 Signed-off-by: Shiju --- Cargo.lock | 2 + crates/openshell-cli/Cargo.toml | 1 + crates/openshell-cli/src/main.rs | 11 ++ crates/openshell-cli/src/run.rs | 29 +++ .../tests/policy_lint_integration.rs | 100 ++++++++++ ...olicy_only_process_identity_integration.rs | 45 +++++ crates/openshell-policy/Cargo.toml | 1 + crates/openshell-policy/src/lib.rs | 174 ++++++++++++++++++ crates/openshell-server/src/grpc/mod.rs | 2 +- crates/openshell-server/src/grpc/policy.rs | 30 +-- crates/openshell-server/src/grpc/sandbox.rs | 10 +- .../openshell-server/src/grpc/validation.rs | 26 +-- .../how-it-works/policies/manage-policies.mdx | 15 ++ skills/generate-sandbox-policy/SKILL.md | 5 +- skills/openshell-cli/SKILL.md | 4 + 15 files changed, 410 insertions(+), 45 deletions(-) create mode 100644 crates/openshell-cli/tests/policy_lint_integration.rs diff --git a/Cargo.lock b/Cargo.lock index d178930a4a..e13693df7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4393,6 +4393,7 @@ dependencies = [ "openshell-providers", "openshell-sdk", "openshell-server", + "openshell-supervisor-middleware-builtins", "openshell-tui", "owo-colors", "prost", @@ -4941,6 +4942,7 @@ dependencies = [ "miette", "openshell-core", "openshell-policy-schema", + "prost", "prost-types", "serde", "serde_json", diff --git a/crates/openshell-cli/Cargo.toml b/crates/openshell-cli/Cargo.toml index d7e1c0b8e4..6bbb4d4327 100644 --- a/crates/openshell-cli/Cargo.toml +++ b/crates/openshell-cli/Cargo.toml @@ -23,6 +23,7 @@ openshell-policy = { path = "../openshell-policy" } openshell-providers = { path = "../openshell-providers" } openshell-sdk = { path = "../openshell-sdk" } openshell-tui = { path = "../openshell-tui" } +openshell-supervisor-middleware-builtins = { path = "../openshell-supervisor-middleware-builtins" } serde = { workspace = true } serde_json = { workspace = true } serde_yml = { workspace = true } diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 7411629483..cb79e876f3 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -2155,6 +2155,13 @@ enum DraftCommands { #[derive(Subcommand, Debug)] enum PolicyCommands { + /// Validate an authored policy file offline, without a gateway. + #[command(help_template = LEAF_HELP_TEMPLATE)] + Lint { + /// Path to the authored policy YAML file. + #[arg(value_hint = ValueHint::FilePath)] + policy: String, + }, /// Update policy on a live sandbox. #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Set { @@ -3062,6 +3069,9 @@ async fn run_async() -> Result<()> { // ----------------------------------------------------------- // Top-level policy (was `sandbox policy`) // ----------------------------------------------------------- + Some(Commands::Policy { + command: Some(PolicyCommands::Lint { policy }), + }) => run::sandbox_policy_lint(&policy)?, Some(Commands::Policy { command: Some(policy_cmd), }) => { @@ -3069,6 +3079,7 @@ async fn run_async() -> Result<()> { let mut tls = tls.with_gateway_name(&ctx.name); apply_auth(&mut tls, &ctx.name)?; match policy_cmd { + PolicyCommands::Lint { .. } => unreachable!("offline lint is dispatched above"), PolicyCommands::Set { name, policy, diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index eb224685ed..64521d386c 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5408,6 +5408,35 @@ pub async fn sandbox_upload( // Sandbox policy commands // --------------------------------------------------------------------------- +pub fn sandbox_policy_lint(policy_path: &str) -> Result<()> { + let policy = openshell_policy::parse_sandbox_policy_file(Path::new(policy_path))?; + let mut violations: Vec = openshell_policy::find_endpoint_ambiguities(&policy) + .iter() + .map(ToString::to_string) + .collect(); + let mut middlewares: Vec<_> = policy.network_middlewares.iter().collect(); + middlewares.sort_by_key(|(name, _)| *name); + for (name, middleware) in middlewares { + if middleware.middleware.starts_with("openshell/") + && let Err(error) = openshell_supervisor_middleware_builtins::validate_config( + &middleware.middleware, + &middleware.config.clone().unwrap_or_default(), + ) + { + violations.push(format!("middleware config '{name}' is invalid: {error}")); + } + } + if let Err(error) = openshell_policy::validate_and_canonicalize_authored_policy(policy) { + violations.extend(error); + } + if !violations.is_empty() { + miette::bail!("policy is invalid:\n- {}", violations.join("\n- ")); + } + println!("Policy is valid (offline): {policy_path}"); + println!("Gateway checks for providers, external middleware, images and drivers are not run."); + Ok(()) +} + pub async fn sandbox_policy_set_global( server: &str, policy_path: &str, diff --git a/crates/openshell-cli/tests/policy_lint_integration.rs b/crates/openshell-cli/tests/policy_lint_integration.rs new file mode 100644 index 0000000000..8ad15467d3 --- /dev/null +++ b/crates/openshell-cli/tests/policy_lint_integration.rs @@ -0,0 +1,100 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +mod common; + +#[test] +fn policy_lint_runs_offline_and_reports_file_errors() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("policy.yaml"); + let path = file.to_str().unwrap(); + let cases = [ + ( + "version: 1\n", + true, + vec!["Policy is valid (offline)", "Gateway checks"], + ), + ("process: {}\n", false, vec!["version"]), + ("version: 1\nunknown: true\n", false, vec!["unknown"]), + ( + "version: 1\nprocess:\n run_as_user: '0'\n run_as_group: root\nfilesystem_policy:\n read_write: [relative, /]\n", + false, + vec![ + "run_as_user", + "run_as_group", + "path must be absolute", + "overly broad", + ], + ), + ( + "version: 1\nnetwork_policies:\n _provider_private:\n endpoints: []\n", + false, + vec!["reserved '_provider_'", "--base"], + ), + ( + "version: 1\nnetwork_policies:\n bound:\n endpoints:\n - host: example.com\n port: 443\n credential_binding: {provider: ' '}\n", + false, + vec!["credential_binding.provider"], + ), + ( + "version: 1\nprocess:\n run_as_user: '0'\nnetwork_middlewares:\n redact:\n middleware: openshell/regex\n order: 1\n endpoints: {include: [example.com]}\n config: {mode: allow}\n", + false, + vec!["mode: redact", "run_as_user"], + ), + ( + "version: 1\nnetwork_policies:\n conflict:\n endpoints:\n - {host: example.com, port: 443}\n - {host: example.com, port: 443, tls: skip}\n", + false, + vec!["overlap", "conflicting"], + ), + ]; + for (yaml, valid, messages) in cases { + std::fs::write(&file, yaml).unwrap(); + for args in [ + vec!["policy", "lint", path], + vec![ + "--gateway-endpoint", + "http://127.0.0.1:1", + "policy", + "lint", + path, + ], + ] { + let output = common::run_isolated(&args); + assert_eq!(output.code == 0, valid, "{yaml}\n{}", output.combined); + assert!( + !output.combined.contains("failed to connect"), + "{}", + output.combined + ); + for message in &messages { + assert!( + output.combined.contains(message), + "expected {message}: {}", + output.combined + ); + } + } + } + std::fs::write( + &file, + format!( + "version: 1\nnetwork_policies:\n large:\n name: {}\n", + "x".repeat(openshell_policy::MAX_AUTHORED_POLICY_SIZE) + ), + ) + .unwrap(); + let oversized = common::run_isolated(&["policy", "lint", path]); + assert_ne!(oversized.code, 0); + assert!( + oversized + .combined + .contains("policy serialized size exceeds maximum"), + "{}", + oversized.combined + ); + let missing = dir.path().join("missing.yaml"); + assert_ne!( + common::run_isolated(&["policy", "lint", missing.to_str().unwrap()]).code, + 0 + ); +} diff --git a/crates/openshell-cli/tests/policy_only_process_identity_integration.rs b/crates/openshell-cli/tests/policy_only_process_identity_integration.rs index 1b5f4594b0..559534e68a 100644 --- a/crates/openshell-cli/tests/policy_only_process_identity_integration.rs +++ b/crates/openshell-cli/tests/policy_only_process_identity_integration.rs @@ -138,3 +138,48 @@ async fn policy_only_preserves_all_process_identity_combinations_through_gateway } } } + +#[tokio::test] +async fn authored_policy_lint_matches_gateway_file_validation() { + let server = run_server().await; + let cases = [ + ( + "identity", + "process:\n run_as_user: '0'\nfilesystem_policy:\n read_write: [relative]\n", + ), + ("reserved", "network_policies:\n _provider_private: {}\n"), + ( + "binding", + "network_policies:\n bound:\n endpoints:\n - host: example.com\n port: 443\n credential_binding: {provider: ' '}\n", + ), + ]; + for (name, body) in cases { + let policy = + openshell_policy::parse_sandbox_policy(&format!("version: 1\n{body}")).unwrap(); + let violations = + openshell_policy::validate_and_canonicalize_authored_policy(policy.clone()) + .unwrap_err(); + let mut client = grpc_client(&server.endpoint, &server.tls).await.unwrap(); + let error = client + .create_sandbox(CreateSandboxRequest { + name: name.to_string(), + spec: Some(SandboxSpec { + policy: Some(policy), + ..Default::default() + }), + workspace_scope: Some(openshell_core::proto::workspace_selector( + "default".to_string(), + )), + ..Default::default() + }) + .await + .expect_err("invalid authored file must be rejected by the gateway too"); + assert_eq!(error.code(), tonic::Code::InvalidArgument); + for violation in violations { + assert!( + error.message().contains(&violation), + "{name}: {error} did not report {violation}" + ); + } + } +} diff --git a/crates/openshell-policy/Cargo.toml b/crates/openshell-policy/Cargo.toml index 0515808144..c4aea49753 100644 --- a/crates/openshell-policy/Cargo.toml +++ b/crates/openshell-policy/Cargo.toml @@ -18,6 +18,7 @@ serde = { workspace = true } serde_json = { workspace = true } miette = { workspace = true } prost-types = { workspace = true } +prost = { workspace = true } [lints] workspace = true diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index a84bca7868..b92efc5ab0 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -33,6 +33,7 @@ use openshell_core::proto::{ LandlockPolicy, McpOptions, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, ProcessPolicy, SandboxPolicy, }; +use prost::Message; pub use compose::{ PROVIDER_RULE_NAME_PREFIX, ProviderPolicyLayer, compose_effective_policy, @@ -1886,6 +1887,118 @@ pub fn validate_and_canonicalize_sandbox_policy( Ok(policy) } +/// Maximum protobuf-encoded policy size used by sandbox-spec validation and +/// offline linting, in bytes. +pub const MAX_AUTHORED_POLICY_SIZE: usize = 262_144; + +/// Validate and canonicalize authored policy fields without gateway state. +/// +/// Provider-composed policies must use [`validate_and_canonicalize_sandbox_policy`] +/// instead: their generated rule names are not authored input. Endpoint ambiguity +/// and implementation-owned middleware configuration use their shared validators +/// separately, including when effective provider layers are available. +pub fn validate_and_canonicalize_authored_policy( + policy: SandboxPolicy, +) -> std::result::Result> { + let mut violations = validate_authored_rule_names(&policy) + .err() + .unwrap_or_default(); + if let Err(errors) = validate_credential_bindings(&policy) { + violations.extend(errors); + } + let original_size = policy.encoded_len(); + if original_size > MAX_AUTHORED_POLICY_SIZE { + violations.push(format!( + "policy serialized size exceeds maximum ({original_size} > {MAX_AUTHORED_POLICY_SIZE})" + )); + } + let policy = match validate_and_canonicalize_sandbox_policy(policy) { + Ok(policy) => policy, + Err(error) => { + violations.extend(error.violations().iter().map(ToString::to_string)); + return Err(violations); + } + }; + let size = policy.encoded_len(); + if original_size <= MAX_AUTHORED_POLICY_SIZE && size > MAX_AUTHORED_POLICY_SIZE { + violations.push(format!( + "policy serialized size exceeds maximum ({size} > {MAX_AUTHORED_POLICY_SIZE})" + )); + } + if violations.is_empty() { + Ok(policy) + } else { + Err(violations) + } +} + +/// Reject authored rule keys reserved for provider composition. +pub fn validate_authored_rule_names( + policy: &SandboxPolicy, +) -> std::result::Result<(), Vec> { + let mut violations = Vec::new(); + let mut names: Vec<_> = policy.network_policies.keys().collect(); + names.sort(); + for name in names { + if is_provider_rule_name(name) { + violations.push(format!( + "network_policies key '{name}' uses reserved '_provider_' prefix for provider composition; use 'openshell policy get --base' for a round-trippable base policy, or use 'openshell policy get --full' to inspect the effective policy including provider entries" + )); + } + } + if violations.is_empty() { + Ok(()) + } else { + Err(violations) + } +} + +/// Validate credential-binding structure without resolving attached providers. +pub fn validate_credential_bindings( + policy: &SandboxPolicy, +) -> std::result::Result<(), Vec> { + let mut violations = Vec::new(); + let mut rules: Vec<_> = policy.network_policies.iter().collect(); + rules.sort_by_key(|(name, _)| *name); + for (_, rule) in rules { + for endpoint in &rule.endpoints { + let Some(binding) = &endpoint.credential_binding else { + continue; + }; + let provider = binding.provider.trim(); + if provider.is_empty() { + violations.push(format!( + "credential_binding.provider is required for endpoint '{}'", + endpoint.host + )); + } else if provider != binding.provider { + violations.push(format!("credential_binding.provider '{}' must not contain leading or trailing whitespace", binding.provider)); + } + if endpoint.host.trim().is_empty() { + violations.push(format!( + "credential-bound endpoint for provider '{provider}' must define a host" + )); + } + let invalid_port = if endpoint.ports.is_empty() { + !(1..=u32::from(u16::MAX)).contains(&endpoint.port) + } else { + endpoint + .ports + .iter() + .any(|port| !(1..=u32::from(u16::MAX)).contains(port)) + }; + if invalid_port { + violations.push(format!("credential-bound endpoint '{}' for provider '{provider}' must define ports in range 1..=65535", endpoint.host)); + } + } + } + if violations.is_empty() { + Ok(()) + } else { + Err(violations) + } +} + /// Replace absent protobuf MCP options and empty revision lists with the /// single pinned policy default while preserving every explicit MCP option. pub(crate) fn materialize_default_mcp_versions(policy: &mut SandboxPolicy) { @@ -1950,6 +2063,67 @@ pub use openshell_policy_schema::normalize_path; mod tests { use super::*; + #[test] + fn authored_policy_collects_gateway_file_violations() { + let policy = parse_sandbox_policy( + "version: 1\nprocess:\n run_as_user: '0'\nfilesystem_policy:\n read_write: [relative]\nnetwork_policies:\n _provider_private:\n endpoints:\n - host: example.com\n port: 443\n credential_binding: {provider: ' '}\n", + ).unwrap(); + let errors = validate_and_canonicalize_authored_policy(policy) + .unwrap_err() + .join("; "); + for message in [ + "run_as_user", + "path must be absolute", + "reserved '_provider_'", + "credential_binding.provider", + ] { + assert!(errors.contains(message), "missing {message}: {errors}"); + } + let valid = parse_sandbox_policy("version: 1\n").unwrap(); + assert!(validate_and_canonicalize_authored_policy(valid).is_ok()); + } + + #[test] + fn authored_policy_checks_canonical_encoded_size() { + let mut policy = SandboxPolicy { + version: 1, + network_policies: HashMap::from([( + "mcp".into(), + NetworkPolicyRule { + endpoints: vec![NetworkEndpoint { + host: "mcp.example.com".into(), + port: 443, + protocol: "mcp".into(), + mcp: Some(McpOptions { + allow_all_known_mcp_methods: Some(true), + ..Default::default() + }), + ..Default::default() + }], + name: "x".repeat(MAX_AUTHORED_POLICY_SIZE - 100), + ..Default::default() + }, + )]), + ..Default::default() + }; + // Fill the remaining space without assuming a fixed protobuf layout. + while policy.encoded_len() < MAX_AUTHORED_POLICY_SIZE { + policy + .network_policies + .get_mut("mcp") + .unwrap() + .name + .push('x'); + } + assert_eq!(policy.encoded_len(), MAX_AUTHORED_POLICY_SIZE); + let errors = validate_and_canonicalize_authored_policy(policy) + .unwrap_err() + .join("; "); + assert!( + errors.contains("policy serialized size exceeds maximum"), + "{errors}" + ); + } #[test] fn truncate_for_display_handles_multi_byte_utf8_without_panicking() { // Byte index 77 falls inside the multi-byte 'é'. diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 69208348de..f74d8bdf75 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -174,7 +174,7 @@ const MAX_METADATA_ANNOTATIONS_ENTRIES: usize = 128; /// Maximum serialized size (bytes) for template Struct fields. const MAX_TEMPLATE_STRUCT_SIZE: usize = 65_536; /// Maximum serialized size (bytes) for the policy field. -const MAX_POLICY_SIZE: usize = 262_144; +const MAX_POLICY_SIZE: usize = openshell_policy::MAX_AUTHORED_POLICY_SIZE; /// Maximum length for a provider type slug. const MAX_PROVIDER_TYPE_LEN: usize = 64; /// Maximum number of entries in the provider `credentials` map. diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index cc83789417..26f2787555 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -2153,43 +2153,19 @@ fn policy_static_credential_endpoint_bindings( return Ok(bindings); }; + openshell_policy::validate_credential_bindings(policy) + .map_err(|violations| Status::invalid_argument(violations.join("; ")))?; for rule in policy.network_policies.values() { for endpoint in &rule.endpoints { let Some(binding) = endpoint.credential_binding.as_ref() else { continue; }; - let provider = binding.provider.trim(); - if provider.is_empty() { - return Err(Status::invalid_argument(format!( - "credential_binding.provider is required for endpoint '{}'", - endpoint.host - ))); - } - if provider != binding.provider { - return Err(Status::invalid_argument(format!( - "credential_binding.provider '{}' must not contain leading or trailing whitespace", - binding.provider - ))); - } - if endpoint.host.trim().is_empty() { - return Err(Status::invalid_argument(format!( - "credential-bound endpoint for provider '{provider}' must define a host" - ))); - } + let provider = binding.provider.as_str(); let ports = if endpoint.ports.is_empty() { vec![endpoint.port] } else { endpoint.ports.clone() }; - if ports - .iter() - .any(|port| *port == 0 || *port > u32::from(u16::MAX)) - { - return Err(Status::invalid_argument(format!( - "credential-bound endpoint '{}' for provider '{provider}' must define ports in range 1..=65535", - endpoint.host - ))); - } let provider_bindings = bindings.entry(provider.to_string()).or_default(); for port in ports { let candidate = StaticCredentialEndpointBinding { diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 86ff12b170..34c08926b4 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -69,9 +69,9 @@ use super::provider::{ get_provider_record, is_valid_env_key, validate_provider_environment_keys_unique_with_catalog, }; use super::validation::{ - level_matches, source_matches, validate_and_canonicalize_policy, validate_dns1123_label, - validate_exec_request_fields, validate_no_reserved_provider_policy_keys, - validate_policy_safety, validate_sandbox_governance_spec, validate_sandbox_spec, + level_matches, source_matches, validate_and_canonicalize_authored_policy, + validate_dns1123_label, validate_exec_request_fields, validate_policy_safety, + validate_sandbox_governance_spec, validate_sandbox_spec, }; use super::{MAX_PROVIDERS, MAX_ROUTABLE_NAME_LEN}; use crate::persistence::current_time_ms; @@ -680,8 +680,7 @@ async fn handle_create_sandbox_inner( if let Some(ref mut policy) = spec.policy { super::policy::clear_provider_credentialed_markers(policy); - validate_no_reserved_provider_policy_keys(policy)?; - *policy = validate_and_canonicalize_policy(policy.clone())?; + *policy = validate_and_canonicalize_authored_policy(policy.clone())?; } // Process identity and MCP default materialization can increase the @@ -4290,6 +4289,7 @@ mod tests { assert_redacted, audited, }; use crate::compute::NoopTestDriver; + use crate::grpc::validation::validate_and_canonicalize_policy; #[test] fn exec_requires_ssh_channel_close_after_exit_status() { diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index d84488bcca..16877d1fb6 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -1052,20 +1052,24 @@ pub(super) fn validate_and_canonicalize_policy( }) } -/// Validate that user-authored policy does not use provider-derived rule keys. +/// Validate file-only authored policy rules before applying gateway context. +pub(super) fn validate_and_canonicalize_authored_policy( + policy: ProtoSandboxPolicy, +) -> Result { + openshell_policy::validate_and_canonicalize_authored_policy(policy).map_err(|error| { + Status::invalid_argument(format!( + "policy contains unsafe content: {}", + error.join("; ") + )) + }) +} + +/// Validate that authored rules do not use provider-derived keys. pub(super) fn validate_no_reserved_provider_policy_keys( policy: &ProtoSandboxPolicy, ) -> Result<(), Status> { - if let Some(key) = policy - .network_policies - .keys() - .find(|key| openshell_policy::is_provider_rule_name(key)) - { - return Err(Status::invalid_argument(format!( - "network_policies key '{key}' uses reserved '_provider_' prefix for provider composition; use 'openshell policy get --base' for a round-trippable base policy, or use 'openshell policy get --full' to inspect the effective policy including provider entries" - ))); - } - Ok(()) + openshell_policy::validate_authored_rule_names(policy) + .map_err(|violations| Status::invalid_argument(violations.join("; "))) } /// Validate that static policy fields (filesystem, landlock, process) haven't changed diff --git a/docs/how-it-works/policies/manage-policies.mdx b/docs/how-it-works/policies/manage-policies.mdx index c20af7b67b..6d1d7eb8f9 100644 --- a/docs/how-it-works/policies/manage-policies.mdx +++ b/docs/how-it-works/policies/manage-policies.mdx @@ -13,6 +13,21 @@ setting a policy when you create a sandbox to changing it while the sandbox runs, confirming that the change took effect, and rolling it back. Run `openshell policy --help` for every command and option. +## Check a Policy File Offline + +Run `openshell policy lint policy.yaml` before applying an authored policy. +It validates the file locally and exits nonzero for an invalid policy. No +gateway or sandbox is required. Schema and conversion errors stop validation; +after parsing, safety, credential-binding, ambiguity and built-in middleware +errors are reported together. + +A successful lint uses this CLI version's validation rules. It does not check +attached providers, registered external middleware, image identity, or driver +requirements, and does not establish that the sandbox will start. To validate +an exported policy for reuse, export the authored policy with +`openshell policy get my-sandbox --base`; `--full` includes reserved provider +entries. + ## Create a Sandbox with a Policy Pass a policy file when you create a sandbox. Filesystem, Landlock, and process diff --git a/skills/generate-sandbox-policy/SKILL.md b/skills/generate-sandbox-policy/SKILL.md index 864e9d61f3..8c75cd4c95 100644 --- a/skills/generate-sandbox-policy/SKILL.md +++ b/skills/generate-sandbox-policy/SKILL.md @@ -391,7 +391,10 @@ If the user needs access to multiple hosts or the same host with different rules ## Step 6: Validate and Warn -Before presenting the policy to the user, verify correctness **and** flag breadth concerns. +Before presenting the policy to the user, run `openshell policy lint policy.yaml` +to validate the authored file offline, then flag breadth concerns. Provider +attachments, external middleware registrations, images and drivers still need +gateway checks. ### Hard Errors (would block sandbox startup) diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index b757172b7a..588b9d4c17 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -670,6 +670,10 @@ Create sandbox with initial policy Verify reload succeeded ─────────┘ ``` +Validate an authored policy offline with `openshell policy lint policy.yaml` +before creating or updating a sandbox. Gateway-dependent provider, middleware +registration, image and driver checks are deferred until application. + ### Step 1: Create sandbox with initial policy ```bash From 41daeae1f755b5f7e0ef199e2097c9ee70c1a3f9 Mon Sep 17 00:00:00 2001 From: Shiju Date: Sat, 7 Nov 2026 23:00:36 +0530 Subject: [PATCH 2/2] fix(policy): preserve gateway size errors and refresh example locks Signed-off-by: Shiju --- crates/openshell-server/src/grpc/sandbox.rs | 27 +++++++++++++++---- .../openshell-server/src/grpc/validation.rs | 12 --------- examples/governance-interceptor/Cargo.lock | 1 + .../Cargo.lock | 1 + 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 34c08926b4..d641725159 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -69,9 +69,9 @@ use super::provider::{ get_provider_record, is_valid_env_key, validate_provider_environment_keys_unique_with_catalog, }; use super::validation::{ - level_matches, source_matches, validate_and_canonicalize_authored_policy, - validate_dns1123_label, validate_exec_request_fields, validate_policy_safety, - validate_sandbox_governance_spec, validate_sandbox_spec, + level_matches, source_matches, validate_and_canonicalize_policy, validate_dns1123_label, + validate_exec_request_fields, validate_no_reserved_provider_policy_keys, + validate_policy_safety, validate_sandbox_governance_spec, validate_sandbox_spec, }; use super::{MAX_PROVIDERS, MAX_ROUTABLE_NAME_LEN}; use crate::persistence::current_time_ms; @@ -680,7 +680,8 @@ async fn handle_create_sandbox_inner( if let Some(ref mut policy) = spec.policy { super::policy::clear_provider_credentialed_markers(policy); - *policy = validate_and_canonicalize_authored_policy(policy.clone())?; + validate_no_reserved_provider_policy_keys(policy)?; + *policy = validate_and_canonicalize_policy(policy.clone())?; } // Process identity and MCP default materialization can increase the @@ -4289,7 +4290,6 @@ mod tests { assert_redacted, audited, }; use crate::compute::NoopTestDriver; - use crate::grpc::validation::validate_and_canonicalize_policy; #[test] fn exec_requires_ssh_channel_close_after_exit_status() { @@ -7453,6 +7453,8 @@ mod tests { #[tokio::test] async fn create_sandbox_bounds_canonical_mcp_policy_size_for_all_default_spellings() { + use openshell_core::rpc_error::StatusExt as _; + let state = test_server_state().await; let max_policy_size = super::super::MAX_POLICY_SIZE; @@ -7536,6 +7538,21 @@ mod tests { } else { let error = result.expect_err("oversized canonical policy must be rejected"); assert_eq!(error.code(), tonic::Code::InvalidArgument, "{sandbox_name}"); + let details = error.get_error_details(); + assert_eq!( + details + .bad_request() + .expect("field violation") + .field_violations[0] + .field, + "spec.policy", + "{sandbox_name}" + ); + assert_eq!( + details.error_info().expect("error reason").reason, + "INVALID_ARGUMENT", + "{sandbox_name}" + ); assert_eq!( error.message(), format!( diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 16877d1fb6..0cfc1550d1 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -1052,18 +1052,6 @@ pub(super) fn validate_and_canonicalize_policy( }) } -/// Validate file-only authored policy rules before applying gateway context. -pub(super) fn validate_and_canonicalize_authored_policy( - policy: ProtoSandboxPolicy, -) -> Result { - openshell_policy::validate_and_canonicalize_authored_policy(policy).map_err(|error| { - Status::invalid_argument(format!( - "policy contains unsafe content: {}", - error.join("; ") - )) - }) -} - /// Validate that authored rules do not use provider-derived keys. pub(super) fn validate_no_reserved_provider_policy_keys( policy: &ProtoSandboxPolicy, diff --git a/examples/governance-interceptor/Cargo.lock b/examples/governance-interceptor/Cargo.lock index fa08aa3aba..f8cc4f22bd 100644 --- a/examples/governance-interceptor/Cargo.lock +++ b/examples/governance-interceptor/Cargo.lock @@ -1942,6 +1942,7 @@ dependencies = [ "miette", "openshell-core", "openshell-policy-schema", + "prost", "prost-types", "serde", "serde_json", diff --git a/examples/supervisor-middleware-content-guard/Cargo.lock b/examples/supervisor-middleware-content-guard/Cargo.lock index f453c06919..e40c7de061 100644 --- a/examples/supervisor-middleware-content-guard/Cargo.lock +++ b/examples/supervisor-middleware-content-guard/Cargo.lock @@ -2077,6 +2077,7 @@ dependencies = [ "miette", "openshell-core", "openshell-policy-schema", + "prost", "prost-types", "serde", "serde_json",