From 28495d8803b4380252435ea0d72fed565bbb5611 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Fri, 4 Sep 2026 12:37:08 -0700 Subject: [PATCH] refactor(policy)!: remove NetworkBinary harness field Closes #3054 Signed-off-by: Varsha Prasad Narsing --- architecture/security-policy.md | 28 +- crates/openshell-cli/src/policy_update.rs | 5 +- .../tests/provider_commands_integration.rs | 4 +- crates/openshell-core/src/proto/mod.rs | 90 +++++ crates/openshell-driver-mxc/src/policy.rs | 1 - .../tests/policy_mapper_matrix.rs | 3 - crates/openshell-policy/src/ambiguity.rs | 2 - crates/openshell-policy/src/lib.rs | 32 +- crates/openshell-policy/src/merge.rs | 338 ++++++++++++------ crates/openshell-providers/src/profiles.rs | 69 ++-- .../src/mechanistic_mapper.rs | 17 +- crates/openshell-server/src/grpc/policy.rs | 317 ++++++++++++---- crates/openshell-server/src/grpc/provider.rs | 4 +- crates/openshell-server/src/policy_store.rs | 29 +- .../data/sandbox-policy.rego | 32 +- .../openshell-supervisor-network/src/opa.rs | 100 +----- .../src/policy_local.rs | 22 +- docs/about/release-notes.mdx | 8 + docs/providers/profiles.mdx | 2 +- proto/sandbox.proto | 4 +- sdk/go/proto/sandboxv1/sandbox.pb.go | 21 +- 21 files changed, 687 insertions(+), 441 deletions(-) diff --git a/architecture/security-policy.md b/architecture/security-policy.md index 62f5837e70..6c5d7f77a2 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -281,21 +281,23 @@ After any successful policy write, pending chunks already covered by the new live effective policy are rejected as redundant. This keeps the review inbox aligned with what the sandbox currently enforces. -Endpoint and binary advisor markers are provenance, not authorization or -connection metadata. Provider- or user-authored declarations carry explicit -provenance; `policy.local` declarations carry advisor provenance. A difference -in endpoint provenance alone is compatible during effective-policy ambiguity -validation. When identical endpoint or binary identities merge, an explicit -declaration dominates an advisor declaration. Proposal coverage likewise -ignores provenance so an approved overlay converges when an existing explicit -declaration already supplies the same identity. +Endpoint advisor markers are provenance, not authorization or connection +metadata. Provider- or user-authored endpoints carry explicit provenance; +`policy.local` endpoints carry advisor provenance. A difference in endpoint +provenance alone is compatible during effective-policy ambiguity validation. +When identical endpoints merge, an explicit declaration dominates an advisor +declaration. Proposal coverage likewise ignores provenance so an approved +overlay converges when an existing explicit declaration already supplies the +same identity. This compatibility does not weaken SSRF classification. Exact-host trust -requires one matching rule to contain both an exact explicit endpoint and an -explicit binary identity. An advisor-only endpoint or binary cannot assemble -that trust from unrelated rules. A provider rule may independently establish -trust for its own explicit endpoint and binary pair, but an advisor overlay -does not broaden that pair to a different binary. +requires one matching rule to contain both an exact explicit endpoint and a +matching binary identity. An advisor endpoint cannot assemble that trust from +an unrelated explicit endpoint. When the advisor observes a new binary for an +existing explicit endpoint contract, canonicalization keeps the observation in +a separate rule whose endpoint retains advisor provenance. A provider or user +rule may independently establish trust for its own explicit endpoint and binary +pair, but an advisor overlay does not broaden that pair to a different binary. ### Security-notes gate diff --git a/crates/openshell-cli/src/policy_update.rs b/crates/openshell-cli/src/policy_update.rs index d6ba02e40c..93886725af 100644 --- a/crates/openshell-cli/src/policy_update.rs +++ b/crates/openshell-cli/src/policy_update.rs @@ -60,10 +60,7 @@ pub fn build_policy_update_plan( endpoints: vec![endpoint.clone()], binaries: deduped_binaries .iter() - .map(|path| NetworkBinary { - path: path.clone(), - ..Default::default() - }) + .map(|path| NetworkBinary { path: path.clone() }) .collect(), }; merge_operations.push(PolicyMergeOperation { diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index e48ca84af0..963fcc228a 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -2551,7 +2551,6 @@ binaries: [/usr/bin/yaml-client] } #[tokio::test] -#[allow(deprecated)] async fn provider_profile_import_preserves_advanced_network_policy_fields() { let ts = run_server().await; let dir = tempfile::tempdir().unwrap(); @@ -2580,7 +2579,6 @@ endpoints: path: /v1 binaries: - path: /usr/bin/advanced - harness: true ", ) .unwrap(); @@ -2609,7 +2607,7 @@ binaries: assert_eq!(endpoint.allowed_ips, vec!["10.0.0.0/24"]); assert!(endpoint.allow_encoded_slash); assert_eq!(endpoint.path, "/v1"); - assert!(profile.binaries[0].harness); + assert_eq!(profile.binaries[0].path, "/usr/bin/advanced"); } #[tokio::test] diff --git a/crates/openshell-core/src/proto/mod.rs b/crates/openshell-core/src/proto/mod.rs index d3b3405813..9a9b5db0ef 100644 --- a/crates/openshell-core/src/proto/mod.rs +++ b/crates/openshell-core/src/proto/mod.rs @@ -78,3 +78,93 @@ pub use middleware::v1::*; pub use openshell::*; pub use sandbox::v1::*; pub use test::ObjectForTest; + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use prost::Message; + + use super::SandboxPolicy; + + #[derive(Clone, PartialEq, Message)] + struct LegacyNetworkBinary { + #[prost(string, tag = "1")] + path: String, + #[prost(bool, tag = "2")] + harness: bool, + } + + #[derive(Clone, PartialEq, Message)] + struct LegacyNetworkPolicyRule { + #[prost(message, repeated, tag = "3")] + binaries: Vec, + } + + #[derive(Clone, PartialEq, Message)] + struct LegacySandboxPolicy { + #[prost(map = "string, message", tag = "5")] + network_policies: HashMap, + } + + #[test] + fn sandbox_policy_ignores_removed_network_binary_harness_wire_field() { + let legacy = LegacySandboxPolicy { + network_policies: HashMap::from([( + "legacy".to_string(), + LegacyNetworkPolicyRule { + binaries: vec![LegacyNetworkBinary { + path: "/usr/bin/curl".to_string(), + harness: true, + }], + }, + )]), + }; + + let decoded = SandboxPolicy::decode(legacy.encode_to_vec().as_slice()) + .expect("legacy policy should decode"); + assert_eq!( + decoded.network_policies["legacy"].binaries[0].path, + "/usr/bin/curl" + ); + + let round_tripped = + LegacySandboxPolicy::decode(decoded.encode_to_vec().as_slice()).unwrap(); + assert!(!round_tripped.network_policies["legacy"].binaries[0].harness); + } + + #[test] + fn network_binary_reserves_removed_harness_name_and_tag() { + let descriptor = prost_types::FileDescriptorSet::decode(crate::FILE_DESCRIPTOR_SET) + .expect("descriptor set should decode"); + let network_binary = descriptor + .file + .iter() + .find(|file| file.package.as_deref() == Some("openshell.sandbox.v1")) + .and_then(|file| { + file.message_type + .iter() + .find(|message| message.name.as_deref() == Some("NetworkBinary")) + }) + .expect("NetworkBinary descriptor should exist"); + + assert!( + network_binary + .field + .iter() + .all(|field| field.name.as_deref() != Some("harness")) + ); + assert!( + network_binary + .reserved_range + .iter() + .any(|range| range.start == Some(2) && range.end == Some(3)) + ); + assert!( + network_binary + .reserved_name + .iter() + .any(|name| name == "harness") + ); + } +} diff --git a/crates/openshell-driver-mxc/src/policy.rs b/crates/openshell-driver-mxc/src/policy.rs index a909238851..ce0a5a4a34 100644 --- a/crates/openshell-driver-mxc/src/policy.rs +++ b/crates/openshell-driver-mxc/src/policy.rs @@ -271,7 +271,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".into(), - ..Default::default() }], }, ); diff --git a/crates/openshell-driver-mxc/tests/policy_mapper_matrix.rs b/crates/openshell-driver-mxc/tests/policy_mapper_matrix.rs index 39a94bee9b..cc0c648599 100644 --- a/crates/openshell-driver-mxc/tests/policy_mapper_matrix.rs +++ b/crates/openshell-driver-mxc/tests/policy_mapper_matrix.rs @@ -766,11 +766,9 @@ fn b_binaries_error_per_binary() { binaries: vec![ NetworkBinary { path: "/usr/bin/curl".into(), - ..Default::default() }, NetworkBinary { path: "/usr/bin/wget".into(), - ..Default::default() }, ], }, @@ -1248,7 +1246,6 @@ fn handled_fields_inventory() { endpoints: vec![full_ep, single_port_ep], binaries: vec![NetworkBinary { path: "/usr/bin/curl".into(), - ..Default::default() }], }, ); diff --git a/crates/openshell-policy/src/ambiguity.rs b/crates/openshell-policy/src/ambiguity.rs index 2c0b1b1944..2b6c8bbcae 100644 --- a/crates/openshell-policy/src/ambiguity.rs +++ b/crates/openshell-policy/src/ambiguity.rs @@ -726,7 +726,6 @@ mod tests { endpoints: vec![left], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -737,7 +736,6 @@ mod tests { endpoints: vec![right], binaries: vec![NetworkBinary { path: "/usr/bin/bash".to_string(), - ..Default::default() }], }, ); diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 5f3ec4e452..0e9f8d6eb1 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -456,10 +456,6 @@ struct L7DenyRuleDef { #[serde(deny_unknown_fields)] struct NetworkBinaryDef { path: String, - /// Deprecated: ignored. Kept for backward compat with existing YAML files. - #[serde(default, skip_serializing)] - #[allow(dead_code)] - harness: bool, } // --------------------------------------------------------------------------- @@ -897,10 +893,7 @@ fn to_proto(raw: PolicyFile) -> Result { binaries: rule .binaries .into_iter() - .map(|b| NetworkBinary { - path: b.path, - ..Default::default() - }) + .map(|b| NetworkBinary { path: b.path }) .collect(), }; (key, proto_rule) @@ -1053,7 +1046,6 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { .iter() .map(|b| NetworkBinaryDef { path: b.path.clone(), - harness: false, }) .collect(), }; @@ -4875,6 +4867,28 @@ network_policies: assert!(parse_sandbox_policy(yaml).is_err()); } + #[test] + fn parse_rejects_removed_network_binary_harness_field() { + let yaml = r" +version: 1 +network_policies: + legacy: + endpoints: + - host: example.com + port: 443 + binaries: + - path: /usr/bin/curl + harness: true +"; + + let error = parse_sandbox_policy(yaml).expect_err("removed harness field must be rejected"); + let error_debug = format!("{error:?}"); + assert!( + error_debug.contains("unknown field `harness`"), + "unexpected error: {error_debug}" + ); + } + #[test] fn rejects_port_above_65535() { let yaml = r" diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index e6d72d4899..3c38cc0841 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -20,8 +20,11 @@ const DEFAULT_JSON_RPC_MAX_BODY_BYTES: u32 = 64 * 1024; /// has one unambiguous endpoint contract, proposing a second generic L4 /// endpoint loses inspection metadata and can make the effective policy /// ambiguous. Preserve the existing contract instead. Sandbox-owned rules are -/// expanded in place; provider-owned rules remain immutable and are mirrored -/// into the requested sandbox-owned overlay. +/// expanded in place only when they already authorize the observed binaries. +/// A new advisor-observed binary stays in a separate endpoint-provenance-marked +/// rule so it cannot inherit exact-host private-address trust. Provider-owned +/// rules remain immutable and are mirrored into the requested sandbox-owned +/// overlay. pub fn canonicalize_advisor_add_rule( base_policy: &SandboxPolicy, effective_policy: &SandboxPolicy, @@ -84,30 +87,52 @@ pub fn canonicalize_advisor_add_rule( .iter() .filter(|(name, _)| !is_provider_rule_name(name)) .filter_map(|(name, rule)| { - rule.endpoints + (rule.endpoints.iter().any(|endpoint| { + let mut normalized = endpoint.clone(); + normalized.provider_credentialed = false; + normalized.advisor_proposed = false; + normalize_endpoint(&mut normalized); + normalized == contract + }) && incoming_rule + .binaries .iter() - .any(|endpoint| { - let mut normalized = endpoint.clone(); - normalized.provider_credentialed = false; - normalized.advisor_proposed = false; - normalize_endpoint(&mut normalized); - normalized == contract - }) - .then_some(name.clone()) + .all(|binary| binary_scope_covers(rule, binary))) + .then_some(name.clone()) }) .collect::>(); sandbox_owners.sort(); let mut contract = contract; - if sandbox_owners.is_empty() { + let target_name = if let Some(owner) = sandbox_owners.first() { + if let Some(endpoint) = + base_policy.network_policies[owner] + .endpoints + .iter() + .find(|endpoint| { + let mut normalized = (*endpoint).clone(); + normalized.provider_credentialed = false; + normalized.advisor_proposed = false; + normalize_endpoint(&mut normalized); + normalized == contract + }) + { + contract.advisor_proposed = endpoint.advisor_proposed; + } + owner.clone() + } else { // A provider-owned contract is mirrored into a new sandbox-owned // advisor overlay, so retain the incoming proposal provenance. contract.advisor_proposed = incoming_endpoint.advisor_proposed; - } - let target_name = sandbox_owners - .first() - .cloned() - .unwrap_or_else(|| requested_rule_name.to_string()); + let mut candidate = requested_rule_name.to_string(); + let mut suffix = 2_u32; + while base_policy.network_policies.contains_key(&candidate) + || effective_policy.network_policies.contains_key(&candidate) + { + candidate = format!("{requested_rule_name}_{suffix}"); + suffix += 1; + } + candidate + }; let mut canonical = incoming_rule.clone(); canonical.name.clone_from(&target_name); canonical.endpoints = vec![contract]; @@ -1153,10 +1178,13 @@ fn add_rule( incoming_rule.name = rule_name.to_string(); } - // Endpoint-overlap fallback: when a chunk arrives with a new rule_name - // that doesn't already exist, fold it into a same-host/port rule if one - // is present. This is intentional for user-authored policies (incremental - // refinements live under one rule name). + // Endpoint-overlap fallback: when an explicit chunk arrives with a new + // rule_name that doesn't already exist, fold it into a same-host/port rule + // if one is present. This is intentional for user-authored policies + // (incremental refinements live under one rule name). Advisor-proposed + // endpoints must stay on their requested key: folding one into an explicit + // endpoint would clear its provenance and let a newly observed binary + // inherit exact-host private-address trust. // // Provider-injected rules (`_provider_*` — see `compose.rs::provider_rule_name`) // are deliberately EXCLUDED from this fallback. Provider profiles supply a @@ -1171,7 +1199,11 @@ fn add_rule( let requested_key_exists = policy.network_policies.contains_key(rule_name); let target_key = if requested_key_exists { Some(rule_name.to_string()) - } else { + } else if incoming_rule + .endpoints + .iter() + .all(|endpoint| !endpoint.advisor_proposed) + { let mut keys: Vec<_> = policy.network_policies.keys().cloned().collect(); keys.sort(); keys.into_iter() @@ -1184,6 +1216,8 @@ fn add_rule( rules_share_endpoint(existing_rule, &incoming_rule) }) }) + } else { + None }; match target_key { @@ -2054,12 +2088,6 @@ fn expand_access_preset(protocol: &str, access: &str) -> Option> { fn append_unique_binaries(existing: &mut Vec, incoming: &[NetworkBinary]) { let mut seen: HashSet = existing.iter().map(|binary| binary.path.clone()).collect(); for binary in incoming { - if let Some(existing_binary) = existing.iter_mut().find(|item| item.path == binary.path) { - if !is_advisor_proposed_binary(binary) { - mark_user_declared_binary(existing_binary); - } - continue; - } if seen.insert(binary.path.clone()) { existing.push(binary.clone()); } @@ -2118,30 +2146,8 @@ fn dedup_strings(values: &mut Vec) { } fn dedup_binaries(values: &mut Vec) { - let mut deduped: Vec = Vec::with_capacity(values.len()); - for binary in std::mem::take(values) { - if let Some(existing) = deduped.iter_mut().find(|item| item.path == binary.path) { - if !is_advisor_proposed_binary(&binary) { - mark_user_declared_binary(existing); - } - } else { - deduped.push(binary); - } - } - *values = deduped; -} - -fn is_advisor_proposed_binary(binary: &NetworkBinary) -> bool { - #[allow(deprecated)] - let advisor_proposed = binary.harness; - advisor_proposed -} - -fn mark_user_declared_binary(binary: &mut NetworkBinary) { - #[allow(deprecated)] - { - binary.harness = false; - } + let mut seen = HashSet::new(); + values.retain(|binary| seen.insert(binary.path.clone())); } fn dedup_l7_rules(values: &mut Vec) { @@ -2248,18 +2254,6 @@ mod tests { } } - fn advisor_binary(path: &str) -> NetworkBinary { - let mut binary = NetworkBinary { - path: path.to_string(), - ..Default::default() - }; - #[allow(deprecated)] - { - binary.harness = true; - } - binary - } - fn rest_rule(method: &str, path: &str) -> L7Rule { L7Rule { allow: Some(L7Allow { @@ -2276,7 +2270,7 @@ mod tests { } #[test] - fn canonicalize_advisor_expands_existing_inspected_rule_without_l7_downgrade() { + fn canonicalize_advisor_keeps_new_binary_separate_from_explicit_rule() { let mut existing_endpoint = endpoint("index.crates.io", 443); existing_endpoint.protocol = "rest".to_string(); existing_endpoint.enforcement = "enforce".to_string(); @@ -2286,7 +2280,6 @@ mod tests { endpoints: vec![existing_endpoint.clone()], binaries: vec![NetworkBinary { path: "/usr/bin/cargo".to_string(), - ..Default::default() }], }; let mut base = SandboxPolicy::default(); @@ -2298,7 +2291,7 @@ mod tests { let incoming = NetworkPolicyRule { name: "allow_index_crates_io_443".to_string(), endpoints: vec![observed], - binaries: vec![advisor_binary("/usr/bin/curl")], + binaries: vec![binary("/usr/bin/curl")], }; let (rule_name, canonical) = canonicalize_advisor_add_rule( @@ -2309,13 +2302,116 @@ mod tests { ) .unwrap(); - assert_eq!(rule_name, "cargo_registry"); - assert_eq!(canonical.endpoints, vec![existing_endpoint]); + assert_eq!(rule_name, "allow_index_crates_io_443"); + assert_eq!(canonical.endpoints[0].protocol, existing_endpoint.protocol); + assert_eq!(canonical.endpoints[0].access, existing_endpoint.access); + assert!(canonical.endpoints[0].advisor_proposed); assert_eq!(canonical.binaries[0].path, "/usr/bin/curl"); - #[allow(deprecated)] - { - assert!(canonical.binaries[0].harness); - } + } + + #[test] + fn canonicalize_advisor_avoids_explicit_requested_key_collision() { + let mut base = SandboxPolicy::default(); + base.network_policies.insert( + "allow_index_crates_io_443".to_string(), + NetworkPolicyRule { + name: "explicit-index".to_string(), + endpoints: vec![endpoint("index.crates.io", 443)], + binaries: vec![binary("/usr/bin/cargo")], + }, + ); + let incoming = NetworkPolicyRule { + name: "allow_index_crates_io_443".to_string(), + endpoints: vec![NetworkEndpoint { + host: "index.crates.io".to_string(), + port: 443, + advisor_proposed: true, + ..Default::default() + }], + binaries: vec![binary("/usr/bin/curl")], + }; + + let (rule_name, canonical) = + canonicalize_advisor_add_rule(&base, &base, "allow_index_crates_io_443", &incoming) + .unwrap(); + assert_eq!(rule_name, "allow_index_crates_io_443_2"); + assert!(canonical.endpoints[0].advisor_proposed); + + let merged = merge_policy( + base, + &[PolicyMergeOp::AddRule { + rule_name: rule_name.clone(), + rule: canonical, + }], + ) + .unwrap(); + assert_eq!( + merged.policy.network_policies["allow_index_crates_io_443"].binaries, + vec![binary("/usr/bin/cargo")] + ); + assert!(merged.policy.network_policies[&rule_name].endpoints[0].advisor_proposed); + } + + #[test] + fn canonicalize_advisor_reuses_explicit_rule_for_existing_binary() { + let existing_endpoint = endpoint("index.crates.io", 443); + let existing = NetworkPolicyRule { + name: "cargo-registry".to_string(), + endpoints: vec![existing_endpoint], + binaries: vec![binary("/usr/bin/curl")], + }; + let mut base = SandboxPolicy::default(); + base.network_policies + .insert("cargo_registry".to_string(), existing); + let incoming = NetworkPolicyRule { + name: "allow_index_crates_io_443".to_string(), + endpoints: vec![NetworkEndpoint { + host: "index.crates.io".to_string(), + port: 443, + advisor_proposed: true, + ..Default::default() + }], + binaries: vec![binary("/usr/bin/curl")], + }; + + let (rule_name, canonical) = + canonicalize_advisor_add_rule(&base, &base, "allow_index_crates_io_443", &incoming) + .unwrap(); + + assert_eq!(rule_name, "cargo_registry"); + assert!(!canonical.endpoints[0].advisor_proposed); + } + + #[test] + fn canonicalize_advisor_preserves_existing_advisor_endpoint_provenance() { + let mut advisor_endpoint = endpoint("index.crates.io", 443); + advisor_endpoint.advisor_proposed = true; + let mut base = SandboxPolicy::default(); + base.network_policies.insert( + "advisor_index".to_string(), + NetworkPolicyRule { + name: "advisor-index".to_string(), + endpoints: vec![advisor_endpoint], + binaries: vec![binary("/usr/bin/curl")], + }, + ); + let incoming = NetworkPolicyRule { + name: "allow_index_crates_io_443".to_string(), + endpoints: vec![NetworkEndpoint { + host: "index.crates.io".to_string(), + port: 443, + advisor_proposed: true, + ..Default::default() + }], + binaries: vec![binary("/usr/bin/curl")], + }; + + let (rule_name, canonical) = + canonicalize_advisor_add_rule(&base, &base, "allow_index_crates_io_443", &incoming) + .unwrap(); + + assert_eq!(rule_name, "advisor_index"); + assert!(canonical.endpoints[0].advisor_proposed); } #[test] @@ -2343,7 +2439,7 @@ mod tests { advisor_proposed: true, ..Default::default() }], - binaries: vec![advisor_binary("/usr/bin/curl")], + binaries: vec![binary("/usr/bin/curl")], }; let (rule_name, canonical) = @@ -2378,7 +2474,7 @@ mod tests { NetworkPolicyRule { name: "existing-advisor".to_string(), endpoints: vec![advisor_endpoint], - binaries: vec![advisor_binary("/usr/bin/curl")], + binaries: vec![binary("/usr/bin/curl")], }, ); @@ -2400,16 +2496,17 @@ mod tests { advisor_proposed: true, ..Default::default() }], - binaries: vec![advisor_binary("/usr/bin/python")], + binaries: vec![binary("/usr/bin/python")], }; let (rule_name, canonical) = canonicalize_advisor_add_rule(&base, &effective, "advisor_example", &incoming) .expect("provenance alone must not create multiple endpoint contracts"); - assert_eq!(rule_name, "existing_advisor"); + assert_eq!(rule_name, "advisor_example"); assert_eq!(canonical.endpoints[0].protocol, "rest"); assert_eq!(canonical.endpoints[0].access, "read-only"); + assert!(canonical.endpoints[0].advisor_proposed); } #[test] @@ -2437,7 +2534,7 @@ mod tests { advisor_proposed: true, ..Default::default() }], - binaries: vec![advisor_binary("/usr/bin/curl")], + binaries: vec![binary("/usr/bin/curl")], }; let (rule_name, canonical) = canonicalize_advisor_add_rule( @@ -2479,7 +2576,6 @@ mod tests { fn binary(path: &str) -> NetworkBinary { NetworkBinary { path: path.to_string(), - ..Default::default() } } @@ -3663,7 +3759,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -3700,14 +3795,14 @@ mod tests { } #[test] - fn add_rule_user_binary_clears_advisor_marker_for_same_path() { + fn add_rule_deduplicates_binary_path() { let mut policy = restrictive_default_policy(); policy.network_policies.insert( "existing".to_string(), NetworkPolicyRule { name: "existing".to_string(), endpoints: vec![endpoint("api.github.com", 443)], - binaries: vec![advisor_binary("/usr/bin/curl")], + binaries: vec![binary("/usr/bin/curl")], }, ); @@ -3716,7 +3811,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -3731,22 +3825,18 @@ mod tests { let rule = &result.policy.network_policies["existing"]; assert_eq!(rule.binaries.len(), 1); - #[allow(deprecated)] - { - assert!(!rule.binaries[0].harness); - } + assert_eq!(rule.binaries[0].path, "/usr/bin/curl"); } #[test] - fn add_rule_duplicate_binaries_prefer_user_declared_marker() { + fn add_rule_deduplicates_binary_paths_within_incoming_rule() { let incoming = NetworkPolicyRule { name: "incoming".to_string(), endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![ - advisor_binary("/usr/bin/curl"), + binary("/usr/bin/curl"), NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }, ], }; @@ -3762,10 +3852,7 @@ mod tests { let rule = &result.policy.network_policies["github"]; assert_eq!(rule.binaries.len(), 1); - #[allow(deprecated)] - { - assert!(!rule.binaries[0].harness); - } + assert_eq!(rule.binaries[0].path, "/usr/bin/curl"); } #[test] @@ -3778,7 +3865,6 @@ mod tests { endpoints: vec![endpoint("api.example.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/python".to_string(), - ..Default::default() }], }, ); @@ -3792,7 +3878,7 @@ mod tests { advisor_proposed: true, ..Default::default() }], - binaries: vec![advisor_binary("/usr/bin/python")], + binaries: vec![binary("/usr/bin/python")], }; let result = merge_policy( @@ -3806,13 +3892,7 @@ mod tests { let rule = &result.policy.network_policies["app-api"]; assert_eq!(rule.binaries.len(), 1, "binary should still dedupe"); - #[allow(deprecated)] - { - assert!( - !rule.binaries[0].harness, - "existing user binary provenance should be retained" - ); - } + assert_eq!(rule.binaries[0].path, "/usr/bin/python"); let internal_endpoint = rule .endpoints .iter() @@ -4162,7 +4242,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/gh".to_string(), - ..Default::default() }], }, ); @@ -4186,7 +4265,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -4209,7 +4287,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -4239,7 +4316,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -4251,7 +4327,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/git".to_string(), - ..Default::default() }], }, ); @@ -4282,7 +4357,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -4297,7 +4371,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/git".to_string(), - ..Default::default() }], }, ); @@ -4333,7 +4406,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -4352,7 +4424,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -4379,7 +4450,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -4401,7 +4471,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -4424,7 +4493,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/git".to_string(), - ..Default::default() }], }; @@ -4458,7 +4526,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -4523,7 +4590,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/gh".to_string(), - ..Default::default() }], }; let composed = compose_effective_policy( @@ -4554,7 +4620,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let result = merge_policy( @@ -4622,7 +4687,6 @@ mod tests { endpoints: vec![endpoint("api.github.com", 443)], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let result = merge_policy( @@ -4666,6 +4730,42 @@ mod tests { ); } + #[test] + fn add_rule_keeps_advisor_binary_separate_from_explicit_endpoint() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "cargo_registry".to_string(), + NetworkPolicyRule { + name: "cargo-registry".to_string(), + endpoints: vec![endpoint("index.crates.io", 443)], + binaries: vec![binary("/usr/bin/cargo")], + }, + ); + + let mut advisor_endpoint = endpoint("index.crates.io", 443); + advisor_endpoint.advisor_proposed = true; + let result = merge_policy( + policy, + &[PolicyMergeOp::AddRule { + rule_name: "allow_index_crates_io_443".to_string(), + rule: NetworkPolicyRule { + name: "allow_index_crates_io_443".to_string(), + endpoints: vec![advisor_endpoint], + binaries: vec![binary("/usr/bin/curl")], + }, + }], + ) + .expect("advisor rule should remain separate"); + + let explicit = &result.policy.network_policies["cargo_registry"]; + assert!(!explicit.endpoints[0].advisor_proposed); + assert_eq!(explicit.binaries, vec![binary("/usr/bin/cargo")]); + + let advisor = &result.policy.network_policies["allow_index_crates_io_443"]; + assert!(advisor.endpoints[0].advisor_proposed); + assert_eq!(advisor.binaries, vec![binary("/usr/bin/curl")]); + } + fn endpoint_with_ports(host: &str, ports: &[u32]) -> NetworkEndpoint { NetworkEndpoint { host: host.to_string(), diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index 9cdf2257ad..c2d9a52ae3 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -3,8 +3,6 @@ //! Declarative provider type profiles. -#![allow(deprecated)] // NetworkBinary::harness remains in the public proto for compatibility. - use openshell_core::mcp::{DEFAULT_MCP_PROTOCOL_VERSION, McpProtocolVersion}; use openshell_core::proto::{ GraphqlOperation, L7Allow, L7DenyRule, L7QueryMatcher, L7Rule, McpOptions, NetworkBinary, @@ -19,7 +17,6 @@ use openshell_policy::{ L7EndpointFields, L7Protocol, validate_explicit_tcp_additional_fields, validate_l7_endpoint_semantics, }; -use serde::ser::SerializeStruct; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; use std::collections::{BTreeSet, HashMap, HashSet}; use std::net::IpAddr; @@ -591,7 +588,6 @@ pub struct GraphqlOperationProfile { #[derive(Debug, Clone, PartialEq, Eq)] pub struct BinaryProfile { pub path: String, - pub harness: bool, } #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] @@ -1032,13 +1028,7 @@ impl Serialize for BinaryProfile { where S: Serializer, { - if !self.harness { - return serializer.serialize_str(&self.path); - } - let mut state = serializer.serialize_struct("BinaryProfile", 2)?; - state.serialize_field("path", &self.path)?; - state.serialize_field("harness", &self.harness)?; - state.end() + serializer.serialize_str(&self.path) } } @@ -1057,19 +1047,23 @@ impl<'de> Deserialize<'de> for BinaryProfile { #[derive(Deserialize)] struct BinaryProfileObject { path: String, - #[serde(default)] - harness: bool, + #[serde(flatten)] + extra: HashMap, } match BinaryProfileInput::deserialize(deserializer)? { - BinaryProfileInput::Path(path) => Ok(Self { - path, - harness: false, - }), - BinaryProfileInput::Object(binary) => Ok(Self { - path: binary.path, - harness: binary.harness, - }), + BinaryProfileInput::Path(path) => Ok(Self { path }), + BinaryProfileInput::Object(binary) => { + if !binary.extra.is_empty() { + let mut fields = binary.extra.keys().cloned().collect::>(); + fields.sort(); + return Err(de::Error::custom(format!( + "unsupported provider profile binary fields: {}; binaries accept only 'path' and the deprecated 'harness' field was removed in 0.1.0", + fields.join(", ") + ))); + } + Ok(Self { path: binary.path }) + } } } } @@ -1579,14 +1573,12 @@ fn canonicalize_mcp_profile_versions(versions: &mut [String]) { fn binary_to_proto(binary: &BinaryProfile) -> NetworkBinary { NetworkBinary { path: binary.path.clone(), - harness: binary.harness, } } fn binary_from_proto(binary: &NetworkBinary) -> BinaryProfile { BinaryProfile { path: binary.path.clone(), - harness: binary.harness, } } @@ -5075,7 +5067,6 @@ endpoints: allow_uninspected_credentials: true binaries: - path: /usr/bin/custom - harness: true ", ) .expect("profile should parse"); @@ -5117,10 +5108,11 @@ binaries: Some("GET") ); assert_eq!(rest_ep.deny_rules[0].method, "POST"); - assert!(proto.binaries[0].harness); + assert_eq!(proto.binaries[0].path, "/usr/bin/custom"); - let reparsed = parse_profile_yaml(&profile_to_yaml(&profile).expect("serialize YAML")) - .expect("serialized profile should parse"); + let serialized = profile_to_yaml(&profile).expect("serialize YAML"); + assert!(serialized.contains("- /usr/bin/custom")); + let reparsed = parse_profile_yaml(&serialized).expect("serialized profile should parse"); let reprotoo = reparsed.to_proto(); assert_eq!(reprotoo.endpoints[0].access, "read-only"); assert_eq!(reprotoo.endpoints[1].rules.len(), 1); @@ -5128,7 +5120,28 @@ binaries: assert_eq!(reprotoo.endpoints[1].ports, vec![443, 8443]); assert!(reprotoo.endpoints[1].allow_uninspected_credentials); assert!(!reprotoo.endpoints[1].provider_credentialed); - assert!(reprotoo.binaries[0].harness); + assert_eq!(reprotoo.binaries[0].path, "/usr/bin/custom"); + } + + #[test] + fn profile_yaml_rejects_removed_binary_harness_field() { + let error = parse_profile_yaml( + r" +id: legacy-binary +display_name: Legacy binary +binaries: + - path: /usr/bin/custom + harness: true +", + ) + .expect_err("removed harness field must be rejected"); + + assert!( + error + .to_string() + .contains("'harness' field was removed in 0.1.0"), + "unexpected error: {error}" + ); } #[test] diff --git a/crates/openshell-sandbox/src/mechanistic_mapper.rs b/crates/openshell-sandbox/src/mechanistic_mapper.rs index 9be5f8e438..0fdc4649ba 100644 --- a/crates/openshell-sandbox/src/mechanistic_mapper.rs +++ b/crates/openshell-sandbox/src/mechanistic_mapper.rs @@ -145,17 +145,9 @@ pub fn generate_proposals(summaries: &[DenialSummary]) -> Vec { let binaries: Vec = if binary.is_empty() { vec![] } else { - let mut proposal_binary = NetworkBinary { + vec![NetworkBinary { path: binary.clone(), - ..Default::default() - }; - // The deprecated harness bit is ignored by policy YAML, but OPA - // maps it to advisor_proposed to preserve the SSRF two-step flow. - #[allow(deprecated)] - { - proposal_binary.harness = true; - } - vec![proposal_binary] + }] }; let proposed_rule = NetworkPolicyRule { @@ -535,10 +527,7 @@ mod tests { assert_eq!(rule.endpoints[0].port, 443); assert_eq!(rule.binaries.len(), 1); assert_eq!(rule.binaries[0].path, "/usr/bin/curl"); - #[allow(deprecated)] - { - assert!(rule.binaries[0].harness); - } + assert!(rule.endpoints[0].advisor_proposed); // No L7 fields when no samples provided. assert!(rule.endpoints[0].protocol.is_empty()); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 7e588fcac8..80045de50a 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -107,6 +107,89 @@ const STORED_POLICY_SOURCE_GLOBAL: &str = "global policy setting"; /// Maximum number of optimistic retry attempts for policy version conflicts. const MERGE_RETRY_LIMIT: usize = 5; +// Private wire-only compatibility types for policy history written before +// 0.1.0. Public generated bindings intentionally reserve NetworkBinary tag 2, +// but stored policies still need its former advisor-provenance value migrated +// before the unknown field is discarded. +#[derive(Clone, PartialEq, Message)] +struct LegacyStoredNetworkBinary { + #[prost(string, tag = "1")] + path: String, + #[prost(bool, tag = "2")] + advisor_proposed: bool, +} + +#[derive(Clone, PartialEq, Message)] +struct LegacyStoredNetworkPolicyRule { + #[prost(string, tag = "1")] + name: String, + #[prost(message, repeated, tag = "2")] + endpoints: Vec, + #[prost(message, repeated, tag = "3")] + binaries: Vec, +} + +#[derive(Clone, PartialEq, Message)] +struct LegacyStoredSandboxPolicy { + #[prost(map = "string, message", tag = "5")] + network_policies: HashMap, +} + +fn decode_stored_policy(payload: &[u8]) -> Result { + let mut policy = ProtoSandboxPolicy::decode(payload)?; + let legacy = LegacyStoredSandboxPolicy::decode(payload)?; + + for (rule_key, legacy_rule) in legacy.network_policies { + let advisor_paths = legacy_rule + .binaries + .into_iter() + .filter(|binary| binary.advisor_proposed) + .map(|binary| binary.path) + .collect::>(); + if advisor_paths.is_empty() { + continue; + } + + let Some(mut explicit_rule) = policy.network_policies.remove(&rule_key) else { + continue; + }; + let mut advisor_rule = explicit_rule.clone(); + advisor_rule + .binaries + .retain(|binary| advisor_paths.contains(&binary.path)); + explicit_rule + .binaries + .retain(|binary| !advisor_paths.contains(&binary.path)); + if advisor_rule.binaries.is_empty() { + policy.network_policies.insert(rule_key, explicit_rule); + continue; + } + for endpoint in &mut advisor_rule.endpoints { + endpoint.advisor_proposed = true; + } + + if explicit_rule.binaries.is_empty() { + policy.network_policies.insert(rule_key, advisor_rule); + continue; + } + + policy + .network_policies + .insert(rule_key.clone(), explicit_rule); + let key_base = format!("{rule_key}__legacy_advisor"); + let mut advisor_key = key_base.clone(); + let mut suffix = 2_u32; + while policy.network_policies.contains_key(&advisor_key) { + advisor_key = format!("{key_base}_{suffix}"); + suffix += 1; + } + advisor_rule.name.clone_from(&advisor_key); + policy.network_policies.insert(advisor_key, advisor_rule); + } + + Ok(policy) +} + fn emit_sandbox_policy_update_success() { openshell_core::telemetry::emit_lifecycle( LifecycleResource::SandboxPolicy, @@ -620,6 +703,24 @@ fn compute_failed_proposal_evaluation_hash( hex::encode(hasher.finalize()) } +fn available_proposal_rule_name( + base_policy: &ProtoSandboxPolicy, + current_effective_policy: &ProtoSandboxPolicy, + requested_rule_name: &str, +) -> String { + let mut candidate = requested_rule_name.to_string(); + let mut suffix = 2_u32; + while base_policy.network_policies.contains_key(&candidate) + || current_effective_policy + .network_policies + .contains_key(&candidate) + { + candidate = format!("{requested_rule_name}_{suffix}"); + suffix += 1; + } + candidate +} + #[allow(clippy::too_many_arguments)] fn evaluate_proposal_candidate( base_policy: &ProtoSandboxPolicy, @@ -631,15 +732,30 @@ fn evaluate_proposal_candidate( validation_context: PolicyMergeValidationContext<'_>, reuse_validation_result: Option<&str>, ) -> ProposalEvaluation { + // The gateway owns advisor provenance. Do not rely on a supervisor or an + // agent-authored proposal to set this internal marker correctly: every + // proposed endpoint must remain ineligible for exact-host private-address + // trust until an explicit declaration or allowed_ips grants that trust. + let mut proposed_rule = proposed_rule.clone(); + for endpoint in &mut proposed_rule.endpoints { + endpoint.advisor_proposed = true; + } + let canonical = if analysis_mode == "mechanistic" { canonicalize_advisor_add_rule( base_policy, current_effective_policy, requested_rule_name, - proposed_rule, + &proposed_rule, ) } else { - Ok((requested_rule_name.to_string(), proposed_rule.clone())) + let rule_name = available_proposal_rule_name( + base_policy, + current_effective_policy, + requested_rule_name, + ); + proposed_rule.name.clone_from(&rule_name); + Ok((rule_name, proposed_rule.clone())) }; let (rule_name, rule) = match canonical { Ok(value) => value, @@ -653,7 +769,7 @@ fn evaluate_proposal_candidate( validation_result: String::new(), review_token: compute_failed_proposal_evaluation_hash( requested_rule_name, - proposed_rule, + &proposed_rule, current_effective_policy, &application_error, ), @@ -5805,7 +5921,7 @@ fn deterministic_policy_hash(policy: &ProtoSandboxPolicy) -> String { fn canonical_policy_record_identity( record: &PolicyRecord, ) -> Result<(ProtoSandboxPolicy, String), Status> { - let decoded = ProtoSandboxPolicy::decode(record.policy_payload.as_slice()) + let decoded = decode_stored_policy(record.policy_payload.as_slice()) .map_err(|error| Status::internal(format!("decode policy revision failed: {error}")))?; let policy = validate_and_canonicalize_stored_policy(decoded, STORED_POLICY_SOURCE_HISTORY)?; let hash = deterministic_policy_hash(&policy); @@ -6969,7 +7085,7 @@ fn decode_policy_from_global_settings( let raw = hex::decode(encoded) .map_err(|e| Status::internal(format!("global policy decode failed: {e}")))?; - let policy = ProtoSandboxPolicy::decode(raw.as_slice()) + let policy = decode_stored_policy(raw.as_slice()) .map_err(|e| Status::internal(format!("global policy protobuf decode failed: {e}")))?; validate_and_canonicalize_stored_policy(policy, STORED_POLICY_SOURCE_GLOBAL).map(Some) } @@ -7092,6 +7208,120 @@ mod tests { request } + #[test] + fn stored_policy_decode_splits_legacy_advisor_binary_provenance() { + #[derive(Clone, PartialEq, Message)] + struct LegacyStoredPolicyRevisionPayload { + #[prost(message, optional, tag = "1")] + policy: Option, + } + + let legacy = LegacyStoredSandboxPolicy { + network_policies: HashMap::from([( + "cargo_registry".to_string(), + LegacyStoredNetworkPolicyRule { + name: "cargo-registry".to_string(), + endpoints: vec![NetworkEndpoint { + host: "index.crates.io".to_string(), + port: 443, + ..Default::default() + }], + binaries: vec![ + LegacyStoredNetworkBinary { + path: "/usr/bin/cargo".to_string(), + advisor_proposed: false, + }, + LegacyStoredNetworkBinary { + path: "/usr/bin/curl".to_string(), + advisor_proposed: true, + }, + ], + }, + )]), + }; + + let wrapped = LegacyStoredPolicyRevisionPayload { + policy: Some(legacy), + } + .encode_to_vec(); + let record = crate::policy_store::policy_record_from_parts( + "revision-1".to_string(), + "sandbox-1".to_string(), + 1, + "loaded".to_string(), + &wrapped, + 1, + ) + .unwrap(); + let decoded = decode_stored_policy(&record.policy_payload).unwrap(); + let explicit = &decoded.network_policies["cargo_registry"]; + assert_eq!(explicit.binaries[0].path, "/usr/bin/cargo"); + assert!(!explicit.endpoints[0].advisor_proposed); + + let advisor = &decoded.network_policies["cargo_registry__legacy_advisor"]; + assert_eq!(advisor.binaries[0].path, "/usr/bin/curl"); + assert!(advisor.endpoints[0].advisor_proposed); + + let round_tripped = decode_stored_policy(&decoded.encode_to_vec()).unwrap(); + assert_eq!(round_tripped, decoded); + } + + #[test] + fn agent_authored_candidate_avoids_explicit_rule_name_collision() { + let mut base = ProtoSandboxPolicy::default(); + base.network_policies.insert( + "allow_index_crates_io_443".to_string(), + NetworkPolicyRule { + name: "explicit-index".to_string(), + endpoints: vec![NetworkEndpoint { + host: "index.crates.io".to_string(), + port: 443, + ..Default::default() + }], + binaries: vec![NetworkBinary { + path: "/usr/bin/cargo".to_string(), + }], + }, + ); + let proposal = NetworkPolicyRule { + name: "allow_index_crates_io_443".to_string(), + endpoints: vec![NetworkEndpoint { + host: "index.crates.io".to_string(), + port: 443, + ..Default::default() + }], + binaries: vec![NetworkBinary { + path: "/usr/bin/curl".to_string(), + }], + }; + + let evaluation = evaluate_proposal_candidate( + &base, + &base, + "allow_index_crates_io_443", + &proposal, + "agent_authored", + &CredentialSet::default(), + PolicyMergeValidationContext { + provider_layers: &[], + credential_binding: None, + }, + None, + ); + + assert!(evaluation.application_error.is_empty()); + assert_eq!(evaluation.rule_name, "allow_index_crates_io_443_2"); + assert!(evaluation.rule.endpoints[0].advisor_proposed); + let candidate = evaluation.candidate_effective_policy.unwrap(); + assert_eq!( + candidate.network_policies["allow_index_crates_io_443"].binaries[0].path, + "/usr/bin/cargo" + ); + assert!( + candidate.network_policies["allow_index_crates_io_443_2"].endpoints[0].advisor_proposed + ); + } + /// Wrap a request with a sandbox `Principal` bound to `sandbox_id`. /// Use for tests that exercise sandbox-caller code paths. #[allow(dead_code)] @@ -9382,7 +9612,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -9505,7 +9734,6 @@ mod tests { } #[tokio::test] - #[allow(deprecated)] async fn provider_policy_layers_include_custom_provider_profiles() { let store = test_store().await; store @@ -9550,7 +9778,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/custom".to_string(), - harness: true, }], inference_capable: false, discovery: None, @@ -9574,7 +9801,7 @@ mod tests { assert_eq!(layers[0].rule.endpoints[0].allowed_ips, vec!["10.0.0.0/24"]); assert!(layers[0].rule.endpoints[0].allow_encoded_slash); assert_eq!(layers[0].rule.endpoints[0].path, "/v1"); - assert!(layers[0].rule.binaries[0].harness); + assert_eq!(layers[0].rule.binaries[0].path, "/usr/bin/custom"); } #[tokio::test] @@ -12066,7 +12293,6 @@ mod tests { } #[tokio::test] - #[allow(deprecated)] async fn custom_imported_profile_policy_and_env_follow_attach_detach_lifecycle() { use crate::grpc::provider::handle_import_provider_profiles; use crate::grpc::sandbox::{ @@ -12115,7 +12341,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/custom".to_string(), - harness: true, }], inference_capable: false, discovery: None, @@ -12478,7 +12703,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: binary.to_string(), - ..Default::default() }], }), ..Default::default() @@ -12500,6 +12724,11 @@ mod tests { .await .unwrap() .unwrap(); + let proposed_rule = NetworkPolicyRule::decode(chunk.proposed_rule.as_slice()).unwrap(); + assert!( + proposed_rule.endpoints[0].advisor_proposed, + "the gateway must stamp advisor endpoint provenance" + ); chunk.validation_result = format!("prover: cached sentinel {index}"); assert!( state @@ -12542,6 +12771,12 @@ mod tests { let policy = ProtoSandboxPolicy::decode(revision.policy_payload.as_slice()).unwrap(); assert!(policy.network_policies.contains_key("alpha")); assert!(policy.network_policies.contains_key("beta")); + assert!( + policy + .network_policies + .values() + .all(|rule| rule.endpoints[0].advisor_proposed) + ); for (index, chunk) in chunks.iter().enumerate() { let stored = state .store @@ -12593,7 +12828,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -12613,7 +12847,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/wget".to_string(), - ..Default::default() }], }), ..Default::default() @@ -12733,7 +12966,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, }, @@ -12748,7 +12980,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/wget".to_string(), - ..Default::default() }], }, }, @@ -12815,7 +13046,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -12921,7 +13151,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let submit = handle_submit_policy_analysis( @@ -13033,7 +13262,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let mut chunk = pending_draft_chunk("legacy-private", sandbox_id); @@ -13106,7 +13334,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -13169,7 +13396,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let first = handle_submit_policy_analysis( @@ -13334,7 +13560,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -13565,7 +13790,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -13686,7 +13910,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -13796,7 +14019,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let mechanistic_submit = handle_submit_policy_analysis( @@ -13874,7 +14096,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let agent_submit = handle_submit_policy_analysis( @@ -14005,7 +14226,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -14072,7 +14292,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/cargo".to_string(), - ..Default::default() }], }, ); @@ -14097,14 +14316,9 @@ mod tests { state.store.put_message(&sandbox).await.unwrap(); seed_sandbox_approval_mode(&state, &sandbox_name, "auto").await; - let mut advisor_binary = NetworkBinary { + let advisor_binary = NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }; - #[allow(deprecated)] - { - advisor_binary.harness = true; - } handle_submit_policy_analysis( &state, with_user(Request::new(SubmitPolicyAnalysisRequest { @@ -14239,7 +14453,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -14311,7 +14524,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -14366,7 +14578,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/wget".to_string(), - ..Default::default() }], }, ); @@ -14443,7 +14654,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let submit = handle_submit_policy_analysis( @@ -14547,7 +14757,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -14651,7 +14860,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -14748,7 +14956,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -14836,7 +15043,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -14928,7 +15134,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -15023,7 +15228,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -15114,7 +15318,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -15179,7 +15382,6 @@ mod tests { endpoints: vec![endpoint], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -15260,7 +15462,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let chunk = DraftChunkRecord { @@ -15381,7 +15582,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -15481,7 +15681,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -15570,7 +15769,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -15663,7 +15861,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], inference_capable: false, discovery: None, @@ -15704,7 +15901,6 @@ mod tests { sandbox.set_phase(SandboxPhase::Ready as i32); state.store.put_message(&sandbox).await.unwrap(); - #[allow(deprecated)] let proposed_rule = NetworkPolicyRule { name: "github_contents_write".to_string(), endpoints: vec![NetworkEndpoint { @@ -15727,7 +15923,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - harness: true, }], }; @@ -15907,7 +16102,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let step1 = handle_submit_policy_analysis( @@ -15948,7 +16142,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let step2 = handle_submit_policy_analysis( @@ -16083,7 +16276,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -16196,7 +16388,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let submit_one = || { @@ -16314,7 +16505,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let submit_one = || { @@ -16578,7 +16768,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -16728,7 +16917,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; @@ -16935,7 +17123,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, }; @@ -16963,7 +17150,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/node".to_string(), - ..Default::default() }], }, }; @@ -16991,7 +17177,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/node".to_string(), - ..Default::default() }], }, }; @@ -17018,7 +17203,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let chunk = DraftChunkRecord { @@ -17088,7 +17272,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, )) @@ -17117,7 +17300,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let chunk = DraftChunkRecord { @@ -17191,7 +17373,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, )) @@ -17220,7 +17401,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }; let chunk = DraftChunkRecord { @@ -18179,7 +18359,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], } } diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 764c0bbfde..ce737c9518 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -6292,7 +6292,6 @@ mod tests { } #[tokio::test] - #[allow(deprecated)] async fn import_provider_profiles_preserves_advanced_proto_policy_fields() { let state = test_server_state().await; let response = handle_import_provider_profiles( @@ -6325,7 +6324,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/advanced".to_string(), - harness: true, }], inference_capable: false, discovery: None, @@ -6368,7 +6366,7 @@ mod tests { ); assert!(endpoint.allow_encoded_slash); assert_eq!(endpoint.path, "/v1"); - assert!(fetched.binaries[0].harness); + assert_eq!(fetched.binaries[0].path, "/usr/bin/advanced"); } #[tokio::test] diff --git a/crates/openshell-server/src/policy_store.rs b/crates/openshell-server/src/policy_store.rs index bd044c8712..072c301265 100644 --- a/crates/openshell-server/src/policy_store.rs +++ b/crates/openshell-server/src/policy_store.rs @@ -11,6 +11,20 @@ use openshell_core::proto::{ use prost::Message; use std::collections::HashMap; +#[derive(Clone, PartialEq, Message)] +struct RawPolicyRevisionPayload { + #[prost(bytes = "vec", optional, tag = "1")] + policy: Option>, + #[prost(string, tag = "2")] + hash: String, + #[prost(string, tag = "3")] + load_error: String, + #[prost(int64, tag = "4")] + loaded_at_ms: i64, + #[prost(map = "string, string", tag = "5")] + provenance: HashMap, +} + #[derive(Debug, Clone)] pub struct AtomicPolicyRevisionWrite { pub id: String, @@ -422,10 +436,10 @@ impl PolicyStoreExt for Store { } pub fn policy_payload_from_record(record: &PolicyRecord) -> PersistenceResult> { - let policy = ProtoSandboxPolicy::decode(record.policy_payload.as_slice()) + ProtoSandboxPolicy::decode(record.policy_payload.as_slice()) .map_err(|e| PersistenceError::Decode(format!("decode policy payload failed: {e}")))?; - Ok(PolicyRevisionPayload { - policy: Some(policy), + Ok(RawPolicyRevisionPayload { + policy: Some(record.policy_payload.clone()), hash: record.policy_hash.clone(), load_error: record.load_error.clone().unwrap_or_default(), loaded_at_ms: record.loaded_at_ms.unwrap_or(0), @@ -442,16 +456,21 @@ pub fn policy_record_from_parts( payload: &[u8], created_at_ms: i64, ) -> PersistenceResult { + let raw_wrapper = RawPolicyRevisionPayload::decode(payload) + .map_err(|e| PersistenceError::Decode(format!("decode raw policy wrapper failed: {e}")))?; let wrapper = PolicyRevisionPayload::decode(payload) .map_err(|e| PersistenceError::Decode(format!("decode policy wrapper failed: {e}")))?; - let policy = wrapper + wrapper + .policy + .ok_or_else(|| PersistenceError::Decode("policy wrapper missing policy".to_string()))?; + let policy_payload = raw_wrapper .policy .ok_or_else(|| PersistenceError::Decode("policy wrapper missing policy".to_string()))?; Ok(PolicyRecord { id, sandbox_id, version, - policy_payload: policy.encode_to_vec(), + policy_payload, policy_hash: wrapper.hash, status, load_error: if wrapper.load_error.is_empty() { diff --git a/crates/openshell-supervisor-network/data/sandbox-policy.rego b/crates/openshell-supervisor-network/data/sandbox-policy.rego index e3fa6d36b4..278291a288 100644 --- a/crates/openshell-supervisor-network/data/sandbox-policy.rego +++ b/crates/openshell-supervisor-network/data/sandbox-policy.rego @@ -171,36 +171,6 @@ binary_allowed(policy, exec) if { glob.match(b.path, ["/"], p) } -user_declared_binary_allowed(_, _) if { - not binary_identity_required -} - -user_declared_binary_allowed(policy, exec) if { - some b - b := policy.binaries[_] - not object.get(b, "advisor_proposed", false) - not contains(b.path, "*") - b.path == exec.path -} - -user_declared_binary_allowed(policy, exec) if { - some b - b := policy.binaries[_] - not object.get(b, "advisor_proposed", false) - not contains(b.path, "*") - ancestor := exec.ancestors[_] - b.path == ancestor -} - -user_declared_binary_allowed(policy, exec) if { - some b in policy.binaries - not object.get(b, "advisor_proposed", false) - contains(b.path, "*") - all_paths := array.concat([exec.path], exec.ancestors) - some p in all_paths - glob.match(b.path, ["/"], p) -} - # --- Network action (allow / deny) --- # # These rules are mutually exclusive by construction: @@ -982,7 +952,7 @@ _policy_has_exact_declared_endpoint(policy) if { exact_declared_endpoint_host if { some pname policy := data.network_policies[pname] - user_declared_binary_allowed(policy, input.exec) + binary_allowed(policy, input.exec) _policy_has_exact_declared_endpoint(policy) } diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 3303aa89b5..81d1c01e2a 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -2098,17 +2098,7 @@ fn proto_to_opa_data_json(proto: &ProtoSandboxPolicy, entrypoint_pid: u32) -> St .binaries .iter() .flat_map(|b| { - // The deprecated harness bit is ignored by policy YAML, but - // advisor-generated proposals use it as internal provenance. - #[allow(deprecated)] - let advisor_proposed = b.harness; - let binary_entry = |path: &str| { - let mut entry = serde_json::json!({"path": path}); - if advisor_proposed { - entry["advisor_proposed"] = true.into(); - } - entry - }; + let binary_entry = |path: &str| serde_json::json!({"path": path}); let mut entries = vec![binary_entry(&b.path)]; match resolve_binary_in_container(&b.path, entrypoint_pid) { BinaryResolution::Resolved(resolved) => { @@ -2250,7 +2240,6 @@ mod tests { ], binaries: vec![NetworkBinary { path: "/usr/local/bin/claude".to_string(), - ..Default::default() }], }, ); @@ -2265,7 +2254,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/glab".to_string(), - ..Default::default() }], }, ); @@ -3359,7 +3347,6 @@ process: }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -3897,7 +3884,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -3969,7 +3955,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -4046,7 +4031,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -4999,7 +4983,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/node".to_string(), - ..Default::default() }], }, ); @@ -5057,7 +5040,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/node".to_string(), - ..Default::default() }], }, ); @@ -5116,7 +5098,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/local/bin/claude".to_string(), - ..Default::default() }], }, ); @@ -5177,7 +5158,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/local/bin/aws".to_string(), - ..Default::default() }], }, ); @@ -5237,7 +5217,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/node".to_string(), - ..Default::default() }], }, ); @@ -5368,7 +5347,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -5383,7 +5361,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/bash".to_string(), - ..Default::default() }], }, ); @@ -6554,64 +6531,6 @@ network_policies: assert!(!engine.query_exact_declared_endpoint_host(&input).unwrap()); } - #[test] - fn exact_declared_endpoint_host_false_for_advisor_proposed_binary() { - let mut network_policies = std::collections::HashMap::new(); - let mut proposal_binary = NetworkBinary { - path: "/usr/bin/curl".to_string(), - ..Default::default() - }; - #[allow(deprecated)] - { - proposal_binary.harness = true; - } - network_policies.insert( - "allow_mcp_internal_corp_example_com_8443".to_string(), - NetworkPolicyRule { - name: "allow_mcp_internal_corp_example_com_8443".to_string(), - endpoints: vec![NetworkEndpoint { - host: "mcp-internal.corp.example.com".to_string(), - port: 8443, - ..Default::default() - }], - binaries: vec![proposal_binary], - }, - ); - let proto = ProtoSandboxPolicy { - version: 1, - filesystem: Some(ProtoFs { - include_workdir: true, - read_only: vec![], - read_write: vec![], - }), - landlock: Some(openshell_core::proto::LandlockPolicy { - compatibility: "best_effort".to_string(), - }), - process: Some(ProtoProc { - run_as_user: "sandbox".to_string(), - run_as_group: "sandbox".to_string(), - }), - network_policies, - network_middlewares: std::collections::HashMap::default(), - }; - let engine = OpaEngine::from_proto(&proto).expect("engine from proto"); - let input = NetworkInput { - host: "mcp-internal.corp.example.com".into(), - port: 8443, - binary_path: PathBuf::from("/usr/bin/curl"), - binary_sha256: "unused".into(), - ancestors: vec![], - cmdline_paths: vec![], - }; - - let decision = engine.evaluate_network(&input).unwrap(); - assert!( - decision.allowed, - "advisor proposal should still allow at OPA L4" - ); - assert!(!engine.query_exact_declared_endpoint_host(&input).unwrap()); - } - #[test] fn exact_declared_endpoint_host_false_for_advisor_proposed_endpoint() { let mut network_policies = std::collections::HashMap::new(); @@ -6628,7 +6547,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/python".to_string(), - ..Default::default() }], }, ); @@ -6699,7 +6617,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -6930,7 +6847,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }, ); @@ -7907,7 +7823,6 @@ network_policies: .iter() .map(|p| NetworkBinary { path: p.to_str().unwrap().to_string(), - ..Default::default() }) .collect(), }, @@ -7967,7 +7882,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/python3".to_string(), - ..Default::default() }], }, ); @@ -8041,7 +7955,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/python3".to_string(), - ..Default::default() }], }, ); @@ -8248,7 +8161,6 @@ network_policies: }], binaries: vec![NetworkBinary { path: "/usr/bin/python3".to_string(), - ..Default::default() }], }, ); @@ -8539,10 +8451,7 @@ network_policies: port: 443, ..Default::default() }], - binaries: vec![NetworkBinary { - path: link_path, - ..Default::default() - }], + binaries: vec![NetworkBinary { path: link_path }], }, ); let proto = ProtoSandboxPolicy { @@ -8617,10 +8526,7 @@ network_policies: port: 443, ..Default::default() }], - binaries: vec![NetworkBinary { - path: link_path, - ..Default::default() - }], + binaries: vec![NetworkBinary { path: link_path }], }, ); let proto = ProtoSandboxPolicy { diff --git a/crates/openshell-supervisor-network/src/policy_local.rs b/crates/openshell-supervisor-network/src/policy_local.rs index 520a22adba..76d2b87c84 100644 --- a/crates/openshell-supervisor-network/src/policy_local.rs +++ b/crates/openshell-supervisor-network/src/policy_local.rs @@ -1110,19 +1110,7 @@ fn network_rule_from_json( let binaries = rule .binaries .into_iter() - .map(|binary| { - let mut proposal_binary = NetworkBinary { - path: binary.path, - ..Default::default() - }; - // The deprecated harness bit is ignored by policy YAML, but OPA - // maps it to advisor_proposed to preserve the SSRF two-step flow. - #[allow(deprecated)] - { - proposal_binary.harness = true; - } - proposal_binary - }) + .map(|binary| NetworkBinary { path: binary.path }) .collect(); Ok(NetworkPolicyRule { @@ -1472,10 +1460,7 @@ mod tests { assert_eq!(rule.endpoints[0].ports, vec![443]); assert_eq!(rule.endpoints[0].protocol, "rest"); assert!(rule.endpoints[0].advisor_proposed); - #[allow(deprecated)] - { - assert!(rule.binaries[0].harness); - } + assert_eq!(rule.binaries[0].path, "/usr/bin/gh"); assert_eq!( rule.endpoints[0].rules[0].allow.as_ref().unwrap().path, "/user/repos" @@ -2020,7 +2005,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }), ..Default::default() @@ -2044,7 +2028,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], } } @@ -2119,7 +2102,6 @@ mod tests { }], binaries: vec![NetworkBinary { path: "/usr/bin/curl".to_string(), - ..Default::default() }], }))); }) diff --git a/docs/about/release-notes.mdx b/docs/about/release-notes.mdx index f763c89952..c5f518fd79 100644 --- a/docs/about/release-notes.mdx +++ b/docs/about/release-notes.mdx @@ -10,6 +10,14 @@ position: 6 NVIDIA OpenShell follows a frequent release cadence. Use the following GitHub resources directly. +## 0.1.0 migration notes + +### Network policy binaries + +OpenShell 0.1.0 removes the deprecated `NetworkBinary.harness` protobuf field and reserves its field number and name. Generated protobuf decoders ignore the unknown value. When the gateway loads policy history written by an older version, it migrates binaries marked by the former field into endpoint-provenance-marked rules so the upgrade cannot widen private-address access. + +Remove `harness` from sandbox policies and provider profiles before upgrading. Policy and profile YAML now reject the property. Provider profiles should list binaries as scalar paths, such as `- /usr/bin/curl`; the transitional object form `- path: /usr/bin/curl` remains accepted and is exported as a scalar. + | Resource | Description | |---|---| | [Releases](https://github.com/NVIDIA/OpenShell/releases) | Versioned release notes and downloadable assets. | diff --git a/docs/providers/profiles.mdx b/docs/providers/profiles.mdx index c310c02b15..cdd41ff777 100644 --- a/docs/providers/profiles.mdx +++ b/docs/providers/profiles.mdx @@ -437,7 +437,7 @@ environment value under the actual environment variable key. `endpoints` contains the same endpoint object shape as sandbox network policy. A profile can use access presets, protocol-specific allow rules, deny rules, WebSocket credential rewriting, request body credential rewriting, GraphQL fields, and SSRF IP allowlists. Because profile credentials are not mapped to individual endpoints, OpenShell conservatively treats every endpoint in a profile that declares credentials as credentialed. Such endpoints require L7 inspection and cannot use `tls: skip` unless the profile explicitly sets `allow_uninspected_credentials: true`. -`binaries` contains the executable paths allowed to reach the profile endpoints when the profile contributes policy to a sandbox. +`binaries` contains the executable paths allowed to reach the profile endpoints when the profile contributes policy to a sandbox. Write each binary as a scalar path. OpenShell also accepts the transitional object form `- path: /usr/bin/example` and exports it as a scalar. The removed `harness` property is rejected; delete it from profiles created before 0.1.0. `inference_capable` marks profiles that are intended to participate in inference workflows. It does not currently mount or configure `inference.local`. diff --git a/proto/sandbox.proto b/proto/sandbox.proto index c2b61d0b3a..894e3b2754 100644 --- a/proto/sandbox.proto +++ b/proto/sandbox.proto @@ -311,8 +311,8 @@ message L7QueryMatcher { // A binary identity for network policy matching. message NetworkBinary { string path = 1; - // Deprecated: the harness concept has been removed. This field is ignored. - bool harness = 2 [deprecated = true]; + reserved 2; + reserved "harness"; } // Request to get sandbox settings by sandbox ID. diff --git a/sdk/go/proto/sandboxv1/sandbox.pb.go b/sdk/go/proto/sandboxv1/sandbox.pb.go index 989589002b..17d8909f83 100644 --- a/sdk/go/proto/sandboxv1/sandbox.pb.go +++ b/sdk/go/proto/sandboxv1/sandbox.pb.go @@ -1439,12 +1439,8 @@ func (x *L7QueryMatcher) GetAny() []string { // A binary identity for network policy matching. type NetworkBinary struct { - state protoimpl.MessageState `protogen:"open.v1"` - Path string `protobuf:"bytes,1,opt,name=path,proto3" json:"path,omitempty"` - // Deprecated: the harness concept has been removed. This field is ignored. - // - // Deprecated: Marked as deprecated in sandbox.proto. - Harness bool `protobuf:"varint,2,opt,name=harness,proto3" json:"harness,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Path string `protobuf:"bytes,1,opt,name=path,proto3" json:"path,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -1486,14 +1482,6 @@ func (x *NetworkBinary) GetPath() string { return "" } -// Deprecated: Marked as deprecated in sandbox.proto. -func (x *NetworkBinary) GetHarness() bool { - if x != nil { - return x.Harness - } - return false -} - // Request to get sandbox settings by sandbox ID. type GetSandboxConfigRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -2196,10 +2184,9 @@ const file_sandbox_proto_rawDesc = "" + "\x05value\x18\x02 \x01(\v2$.openshell.sandbox.v1.L7QueryMatcherR\x05value:\x028\x01J\x04\b\b\x10\t\"6\n" + "\x0eL7QueryMatcher\x12\x12\n" + "\x04glob\x18\x01 \x01(\tR\x04glob\x12\x10\n" + - "\x03any\x18\x02 \x03(\tR\x03any\"A\n" + + "\x03any\x18\x02 \x03(\tR\x03any\"2\n" + "\rNetworkBinary\x12\x12\n" + - "\x04path\x18\x01 \x01(\tR\x04path\x12\x1c\n" + - "\aharness\x18\x02 \x01(\bB\x02\x18\x01R\aharness\"8\n" + + "\x04path\x18\x01 \x01(\tR\x04pathJ\x04\b\x02\x10\x03R\aharness\"8\n" + "\x17GetSandboxConfigRequest\x12\x1d\n" + "\n" + "sandbox_id\x18\x01 \x01(\tR\tsandboxId\"\x19\n" +