diff --git a/.agents/skills/generate-sandbox-policy/SKILL.md b/.agents/skills/generate-sandbox-policy/SKILL.md index 8da14420c9..b70165cf44 100644 --- a/.agents/skills/generate-sandbox-policy/SKILL.md +++ b/.agents/skills/generate-sandbox-policy/SKILL.md @@ -443,7 +443,7 @@ The policy needs to go somewhere. Determine which mode applies: 3. **Apply the change**: - **Adding a new policy**: Insert the new policy block under `network_policies`, maintaining the file's existing indentation and style. - - **Modifying an existing policy**: Edit the specific policy in place — add/remove endpoints, change access presets, update rules, add binaries, etc. + - **Modifying an existing policy**: Edit the specific policy in place — add/remove endpoints, change access presets, update rules, add binaries, etc. A rule authorizes every binary it lists to reach every endpoint and port it lists, so adding one binary grants it all of that rule's endpoints, and adding one endpoint grants it to all of that rule's binaries. State the resulting pairs to the user before writing them. When the user wants a binary to reach only part of a rule's endpoints, put that binary and those endpoints in a separate rule instead of extending the existing one. An empty `binaries` list means any binary, so leaving it off widens the rule to every process. - **Removing a policy**: Delete the policy block if the user asks. 4. **Preserve everything else**: Do not modify `filesystem_policy`, `landlock`, `process`, or other policies unless the user explicitly asks. diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index a36a372df7..37c484c137 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -483,7 +483,22 @@ When denied actions appear: 1. Prefer incremental updates for additive network changes: `openshell policy update work-session --add-endpoint api.github.com:443:read-only:rest:enforce --binary /usr/bin/gh --wait` `openshell policy update work-session --add-allow 'api.github.com:443:POST:/repos/*/issues' --wait` -2. Use full YAML replacement for broad changes or non-network fields: + + A rule authorizes every binary it lists to reach every endpoint it lists, so + an update that adds a binary or an endpoint to an existing rule must declare + that rule's whole binary and endpoint scope. The gateway rejects an update + that would grant a binary-to-endpoint pair the update never asked for, and + the error names the binaries still missing. To grant one binary access to + only part of a rule's endpoints, send the narrow authorization under its own + `--rule-name`; it stays on its own rule instead of folding into the broader + one. + + `--add-allow` and `--add-deny` select an endpoint by host and port alone. If + that host and port appears in more than one rule, or twice in one rule under + different paths, the update is rejected as ambiguous. Fall back to full YAML + replacement for those endpoints. +2. Use full YAML replacement for broad changes or non-network fields, including + any change that would otherwise require restating a large existing scope: `openshell policy get work-session --full > policy.yaml` Modify the policy with the `generate-sandbox-policy` skill. `openshell policy set work-session --policy policy.yaml --wait` diff --git a/crates/openshell-policy/src/merge.rs b/crates/openshell-policy/src/merge.rs index 04f390198d..c89b7842bd 100644 --- a/crates/openshell-policy/src/merge.rs +++ b/crates/openshell-policy/src/merge.rs @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use openshell_core::proto::{ L7Allow, L7DenyRule, L7Rule, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, SandboxPolicy, @@ -9,6 +9,8 @@ use openshell_core::proto::{ use crate::is_provider_rule_name; +const DEFAULT_JSON_RPC_MAX_BODY_BYTES: u32 = 64 * 1024; + #[derive(Debug, Clone, PartialEq)] pub enum PolicyMergeOp { AddRule { @@ -75,6 +77,23 @@ pub enum PolicyMergeWarning { port: u32, incoming: String, }, + /// An `AddRule` named binaries for a rule that already authorizes any + /// binary, so the wider existing scope was kept rather than narrowed. + ExistingAnyBinaryScopeRetained { + rule_name: String, + /// Binary paths the operation named. + incoming: Vec, + }, + /// An `AddRule` operation that would normally fold into an overlapping rule + /// stayed on its requested rule name, because folding would have granted a + /// binary-to-endpoint pair the operation never declared. + KeptRequestedRuleNameToAvoidWidening { + rule_name: String, + /// Rule the endpoint-overlap fallback would otherwise have folded into. + overlapping_rule_name: String, + /// Rendered inheritance conflict that folding would have caused. + reason: String, + }, } impl std::fmt::Display for PolicyMergeWarning { @@ -128,13 +147,102 @@ impl std::fmt::Display for PolicyMergeWarning { f, "endpoint {host}:{port} already uses explicit rules; incoming access preset '{incoming}' was ignored" ), + Self::ExistingAnyBinaryScopeRetained { + rule_name, + incoming, + } => write!( + f, + "rule '{rule_name}' already authorizes any binary; kept that scope instead of narrowing it to {}", + incoming.join(", ") + ), + Self::KeptRequestedRuleNameToAvoidWidening { + rule_name, + overlapping_rule_name, + reason, + } => write!( + f, + "kept add-rule '{rule_name}' on its own rule instead of folding it into overlapping rule '{overlapping_rule_name}': {reason}" + ), } } } +/// Rendered name for the any-binary scope in operator-facing merge errors. An +/// empty binary list on a rule authorizes every binary, which has no path to +/// print. +pub const ANY_BINARY_SCOPE: &str = "any binary"; + #[derive(Debug, Clone, PartialEq, Eq)] pub enum PolicyMergeError { MissingRuleNameForAddRule, + /// An `AddRule` operation has no endpoint authorization to merge. + EmptyAddRuleEndpoints { + operation_index: usize, + rule_name: String, + }, + /// Overlapping endpoints have different effective MCP inspection contracts. + McpContractConflict { + operation_index: usize, + host: String, + port: u32, + /// Rendered effective contract already established at this endpoint. + existing: String, + /// Rendered effective contract the operation asked for. + incoming: String, + }, + /// Newly added binary scope would inherit an existing endpoint + /// authorization that the incoming rule did not declare. + NewBinaryWouldInheritAuthorization { + operation_index: usize, + rule_name: String, + /// Rendered binary scope the operation adds, either a concrete path or + /// [`ANY_BINARY_SCOPE`]. + binary_scope: String, + host: String, + ports: Vec, + }, + /// Existing binaries would inherit a new or changed endpoint authorization, + /// but the incoming rule did not declare the existing binary scope. + ExistingBinariesWouldInheritAuthorization { + operation_index: usize, + rule_name: String, + host: String, + ports: Vec, + /// Existing binary scope the operation must also declare to proceed. + undeclared_binaries: Vec, + }, + /// A widened field would land on a port the operation never named, because + /// the field merges into an endpoint carrying more ports than were declared. + UndeclaredPortWouldChange { + operation_index: usize, + rule_name: String, + host: String, + ports: Vec, + }, + /// One host and port would carry more than one effective MCP inspection + /// contract, which the supervisor resolves by match order rather than policy. + ConflictingMcpContractsForEndpoint { + host: String, + port: u32, + /// Rendered effective contracts found at this host and port, sorted. + contracts: Vec, + }, + /// Several rules carry the same endpoint, so an operation that identifies + /// its target by host and port alone cannot say which binary scope it means. + AmbiguousEndpointRule { + host: String, + port: u32, + /// Rendered rule, and path where one is set, for each endpoint the host + /// and port resolves to. Sorted. + targets: Vec, + }, + /// A remove-binary operation targeted a rule that authorizes any binary, + /// where there is no binary entry to remove. + CannotRemoveBinaryFromAnyBinaryScope { + operation_index: usize, + rule_name: String, + binary_path: String, + }, InvalidEndpointReference { host: String, port: u32, @@ -167,6 +275,79 @@ impl std::fmt::Display for PolicyMergeError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::MissingRuleNameForAddRule => write!(f, "add-rule operation requires a rule name"), + Self::EmptyAddRuleEndpoints { + operation_index, + rule_name, + } => write!( + f, + "merge operation {operation_index} add-rule '{rule_name}' must contain at least one endpoint" + ), + Self::McpContractConflict { + operation_index, + host, + port, + existing, + incoming, + } => write!( + f, + "merge operation {operation_index} cannot combine MCP contracts at {host}:{port}: existing {existing}, incoming {incoming}" + ), + Self::NewBinaryWouldInheritAuthorization { + operation_index, + rule_name, + binary_scope, + host, + ports, + } => write!( + f, + "merge operation {operation_index} add-rule '{rule_name}' would grant {binary_scope} undeclared authorization for {host} on ports {ports:?}" + ), + Self::ExistingBinariesWouldInheritAuthorization { + operation_index, + rule_name, + host, + ports, + undeclared_binaries, + } => write!( + f, + "merge operation {operation_index} add-rule '{rule_name}' would grant existing binaries new or changed authorization for {host} on ports {ports:?}; also declare {}", + undeclared_binaries.join(", ") + ), + Self::UndeclaredPortWouldChange { + operation_index, + rule_name, + host, + ports, + } => write!( + f, + "merge operation {operation_index} add-rule '{rule_name}' would change authorization for {host} on undeclared ports {ports:?}; declare every port of the endpoint it modifies" + ), + Self::ConflictingMcpContractsForEndpoint { + host, + port, + contracts, + } => write!( + f, + "{host}:{port} would carry more than one MCP inspection contract ({}); the sandbox resolves one contract per host and port, so these cannot coexist even in different rules or under different paths", + contracts.join(", ") + ), + Self::AmbiguousEndpointRule { + host, + port, + targets, + } => write!( + f, + "endpoint {host}:{port} resolves to {}; this operation selects its target by host and port alone, so it cannot say which binary scope and L7 surface to widen. Replace the policy with the full YAML instead", + targets.join(", ") + ), + Self::CannotRemoveBinaryFromAnyBinaryScope { + operation_index, + rule_name, + binary_path, + } => write!( + f, + "merge operation {operation_index} cannot remove binary '{binary_path}' from rule '{rule_name}' because the rule authorizes any binary; replace the policy with an explicit binary list or remove the rule" + ), Self::InvalidEndpointReference { host, port } => { write!(f, "invalid endpoint reference '{host}:{port}'") } @@ -213,10 +394,8 @@ pub struct PolicyMergeResult { /// merge of `proposed` would produce. /// /// "Contains" means: for every endpoint in `proposed`, some rule in -/// `policy.network_policies` has an endpoint with overlapping -/// host/path/port set AND containing every L7 allow (method/path) the -/// proposed endpoint requested, and that rule's binaries cover every -/// binary in `proposed`. +/// `policy.network_policies` has an endpoint whose authorization covers the +/// full proposed endpoint and whose binaries cover every proposed binary. /// /// The sandbox's `policy.local /wait` long-poll uses this to decide when /// the local supervisor has actually loaded a policy that includes the @@ -226,59 +405,280 @@ pub struct PolicyMergeResult { /// `/wait` calls (false sleep). This check is the property the agent /// actually cares about — "is my rule in effect right now?". /// -/// L4-vs-L7 split: endpoint overlap reuses `endpoints_overlap` so the -/// L4 surface (host/path/port) lines up with the `add_rule` merge — if -/// the gateway folded the chunk into an existing rule under a different -/// key, this check still returns true. The L7 layer is checked -/// separately because `endpoints_overlap` is intentionally L4-only: -/// without the L7 check, coverage would return true the instant the -/// supervisor reloaded *any* change to an overlapping endpoint, even -/// before the new method/path actually landed — exactly the false-wakeup -/// mode this fix exists to prevent, just one layer down. +/// Coverage is intentionally stricter than endpoint overlap used during +/// merging. Every proposed port and every complete protobuf allow/deny matcher +/// must be loaded. Runtime-defaulted scalars are compared by effective value so +/// omitted defaults do not cause false negatives while explicit policy changes +/// do not become wildcards. Different loaded rules may jointly cover the +/// proposal, but every binary-by-port pair must be present in that union. +/// +/// The atomic authorization unit is one binary reaching one host, path, and +/// port. Ports travel as a set on the wire, but the loaded policy may spread +/// them across rules, so each port resolves independently against the whole +/// union instead of requiring one loaded endpoint to carry them all. +/// +/// Coverage asks whether the loaded policy contains the proposal, not whether +/// the two are identical. The loaded endpoint is a superset of any proposal +/// that merged into an endpoint carrying earlier authorizations, so +/// `endpoint_attributes_cover` compares merge-widened fields by containment, +/// treats an unset proposal value as unspecified for fields the merge retains, +/// and exact-matches only the fields the proposal actually set. pub fn policy_covers_rule(policy: &SandboxPolicy, proposed: &NetworkPolicyRule) -> bool { if proposed.endpoints.is_empty() { return false; } proposed.endpoints.iter().all(|target_endpoint| { - policy.network_policies.values().any(|rule| { - rule.endpoints.iter().any(|endpoint| { - endpoints_overlap(endpoint, target_endpoint) - && endpoint_l7_covers(endpoint, target_endpoint) - }) && proposed.binaries.iter().all(|target_binary| { - rule.binaries - .iter() - .any(|binary| binary.path == target_binary.path) + authorization_ports(target_endpoint) + .into_iter() + .all(|target_port| { + if proposed.binaries.is_empty() { + // An any-binary proposal needs a loaded any-binary scope. A + // rule restricted to named binaries authorizes only those + // binaries, so it can never stand in for "any". + return policy.network_policies.values().any(|rule| { + rule.binaries.is_empty() + && rule_authorizes(rule, target_endpoint, target_port) + }); + } + + proposed.binaries.iter().all(|target_binary| { + policy.network_policies.values().any(|rule| { + binary_scope_covers(rule, target_binary) + && rule_authorizes(rule, target_endpoint, target_port) + }) + }) }) - }) }) } -/// L7 coverage for a single endpoint match. If the proposed endpoint -/// declared explicit L7 allow rules (method+path), every one of them must -/// be present in the merged endpoint's `rules`. An empty `proposed.rules` -/// is treated as "L4-only" and returns true (the endpoint match alone is -/// sufficient). +/// True when some endpoint on `rule` authorizes `proposed` at `port`. +fn rule_authorizes( + rule: &NetworkPolicyRule, + proposed: &NetworkEndpoint, + port: Option, +) -> bool { + rule.endpoints + .iter() + .any(|endpoint| endpoint_authorization_covers_port(endpoint, proposed, port)) +} + +/// The authorization units an endpoint declares, one per port. /// -/// Conservative on access presets: if a merged endpoint uses -/// `access: read-write` instead of explicit rules, this returns false -/// even though the preset would permit the method at runtime. That -/// produces a one-cycle re-issue on the agent's side — preferable to a -/// false-positive coverage signal that lets the agent retry too early. -fn endpoint_l7_covers(merged: &NetworkEndpoint, proposed: &NetworkEndpoint) -> bool { - if proposed.rules.is_empty() { - return true; - } - proposed.rules.iter().all(|proposed_rule| { - let Some(proposed_allow) = proposed_rule.allow.as_ref() else { - return true; - }; - merged.rules.iter().any(|existing| { - existing.allow.as_ref().is_some_and(|existing_allow| { - existing_allow.method == proposed_allow.method - && existing_allow.path == proposed_allow.path - }) +/// `None` represents an endpoint that declares no port at all. Returning it +/// keeps the unit list non-empty, because an empty list would make the callers' +/// `all()` vacuously true and report an endpoint that no loaded rule matches as +/// covered. +fn authorization_ports(endpoint: &NetworkEndpoint) -> Vec> { + let ports = canonical_ports(endpoint); + if ports.is_empty() { + vec![None] + } else { + ports.into_iter().map(Some).collect() + } +} + +fn binary_scope_covers(rule: &NetworkPolicyRule, proposed: &NetworkBinary) -> bool { + rule.binaries.is_empty() + || rule + .binaries + .iter() + .any(|binary| binary.path == proposed.path) +} + +/// Coverage for a single atomic authorization unit: `loaded` authorizes +/// `proposed` at `port`. `None` means the proposal declares no port, which +/// places no constraint on the loaded port set. +fn endpoint_authorization_covers_port( + loaded: &NetworkEndpoint, + proposed: &NetworkEndpoint, + port: Option, +) -> bool { + if let Some(port) = port + && !canonical_ports(loaded).contains(&port) + { + return false; + } + endpoint_attributes_cover(loaded, proposed) +} + +/// Coverage for everything about an endpoint except its ports. +fn endpoint_attributes_cover(loaded: &NetworkEndpoint, proposed: &NetworkEndpoint) -> bool { + // Host and path identify the endpoint rather than describe it: + // `endpoints_overlap` only ever merges endpoints that already agree on + // both, so a difference here is a different endpoint, not an unmet request. + if !loaded.host.eq_ignore_ascii_case(&proposed.host) || loaded.path != proposed.path { + return false; + } + + // Fields `merge_endpoint` retains rather than widens. An unset proposal + // value is unspecified, not a request for the default: the merge leaves the + // loaded value in place and reports success, so comparing effective values + // would report a proposal that did land as uncovered and leave the + // `policy.local /wait` long poll spinning until its deadline. A set value + // still has to match, because the merge would have kept the loaded value + // and the proposal genuinely is not in effect. + if !proposed.protocol.is_empty() && !protocols_match(&loaded.protocol, &proposed.protocol) { + return false; + } + if !proposed.tls.is_empty() && effective_tls(&loaded.tls) != effective_tls(&proposed.tls) { + return false; + } + if !proposed.enforcement.is_empty() + && effective_enforcement(&loaded.enforcement) + != effective_enforcement(&proposed.enforcement) + { + return false; + } + if !proposed.access.is_empty() && loaded.access != proposed.access { + return false; + } + + // The MCP contract is compared unconditionally in both directions because + // `ensure_mcp_contract_compatible` rejects a merge that would give one host + // and port two contracts. A mismatch here therefore cannot be a proposal + // that landed, in either direction. + if !mcp_contracts_match(loaded, proposed) { + return false; + } + + // Widened fields (list appends and `|=` flags) use containment: merging + // into an endpoint that already carries them leaves the loaded copy a + // superset of the proposal, so equality would report "not covered" for a + // proposal that did land. + contains_all(&loaded.rules, &proposed.rules) + && contains_all(&loaded.deny_rules, &proposed.deny_rules) + && contains_all(&loaded.allowed_ips, &proposed.allowed_ips) + && flag_covers(loaded.allow_encoded_slash, proposed.allow_encoded_slash) + && flag_covers( + loaded.websocket_credential_rewrite, + proposed.websocket_credential_rewrite, + ) + && flag_covers( + loaded.request_body_credential_rewrite, + proposed.request_body_credential_rewrite, + ) + && flag_covers(loaded.advisor_proposed, proposed.advisor_proposed) + // Fields the merge neither widens nor retains: it drops them entirely. + // An unset proposal value asks for nothing and is satisfied by whatever + // is loaded; a set value that differs was dropped, so the proposal is + // not in effect and coverage must stay false until it is resubmitted in + // a form the merge preserves. + && unset_or_equal(&proposed.persisted_queries, &loaded.persisted_queries, String::is_empty) + && unset_or_equal( + &proposed.graphql_persisted_queries, + &loaded.graphql_persisted_queries, + HashMap::is_empty, + ) + && unset_or_equal( + &proposed.graphql_max_body_bytes, + &loaded.graphql_max_body_bytes, + |value| *value == 0, + ) + && unset_or_equal( + &proposed.credential_signing, + &loaded.credential_signing, + String::is_empty, + ) + && unset_or_equal( + &proposed.signing_service, + &loaded.signing_service, + String::is_empty, + ) + && unset_or_equal( + &proposed.signing_region, + &loaded.signing_region, + String::is_empty, + ) + && (endpoint_uses_mcp(loaded) + || unset_or_equal( + &proposed.json_rpc_max_body_bytes, + &loaded.json_rpc_max_body_bytes, + |value| *value == 0, + )) +} + +/// Coverage for a field whose proposal value may be unset. An unset proposal +/// value expresses no requirement, so it is covered by any loaded value. +fn unset_or_equal(proposed: &T, loaded: &T, is_unset: impl Fn(&T) -> bool) -> bool { + is_unset(proposed) || proposed == loaded +} + +/// Containment for a field the merge appends to: every proposed entry must be +/// loaded, but the loaded endpoint may carry entries from earlier merges. +fn contains_all(loaded: &[T], proposed: &[T]) -> bool { + proposed.iter().all(|item| loaded.contains(item)) +} + +/// Containment for a field the merge combines with `|=`: the loaded flag only +/// ever gains bits, so coverage holds unless the proposal set a bit that is +/// missing from the loaded endpoint. +fn flag_covers(loaded: bool, proposed: bool) -> bool { + loaded || !proposed +} + +fn protocols_match(left: &str, right: &str) -> bool { + if left.eq_ignore_ascii_case("mcp") || right.eq_ignore_ascii_case("mcp") { + left.eq_ignore_ascii_case("mcp") && right.eq_ignore_ascii_case("mcp") + } else { + left == right + } +} + +fn effective_tls(value: &str) -> &str { + match value { + "" | "terminate" | "passthrough" => "auto", + value => value, + } +} + +fn effective_enforcement(value: &str) -> &str { + if value.is_empty() { "audit" } else { value } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct EffectiveMcpContract { + strict_tool_names: bool, + allow_all_known_mcp_methods: bool, + max_body_bytes: u32, +} + +fn effective_mcp_contract(endpoint: &NetworkEndpoint) -> Option { + endpoint + .protocol + .eq_ignore_ascii_case("mcp") + .then(|| EffectiveMcpContract { + strict_tool_names: endpoint + .mcp + .as_ref() + .and_then(|options| options.strict_tool_names) + .unwrap_or(true), + allow_all_known_mcp_methods: endpoint + .mcp + .as_ref() + .and_then(|options| options.allow_all_known_mcp_methods) + .unwrap_or(false), + max_body_bytes: effective_json_rpc_max_body_bytes(endpoint.json_rpc_max_body_bytes), }) - }) +} + +fn effective_json_rpc_max_body_bytes(value: u32) -> u32 { + if value == 0 { + DEFAULT_JSON_RPC_MAX_BODY_BYTES + } else { + value + } +} + +fn endpoint_uses_mcp(endpoint: &NetworkEndpoint) -> bool { + endpoint.protocol.eq_ignore_ascii_case("mcp") || endpoint.mcp.is_some() +} + +fn mcp_contracts_match(left: &NetworkEndpoint, right: &NetworkEndpoint) -> bool { + match (effective_mcp_contract(left), effective_mcp_contract(right)) { + (None, None) => !endpoint_uses_mcp(left) && !endpoint_uses_mcp(right), + (Some(left), Some(right)) => left == right, + _ => false, + } } pub fn merge_policy( @@ -288,9 +688,15 @@ pub fn merge_policy( let mut merged = policy.clone(); let mut warnings = Vec::new(); - for operation in operations { - apply_operation(&mut merged, operation, &mut warnings)?; + // Validate and apply in request order. `merged` is private until every + // operation succeeds, so failures remain atomic without allowing a later + // malformed operation to replace the error from an earlier operation. + let conflicts_before = conflicting_mcp_host_ports(&policy); + for (operation_index, operation) in operations.iter().enumerate() { + validate_operation(operation_index, operation)?; + apply_operation(&mut merged, operation_index, operation, &mut warnings)?; } + ensure_no_new_mcp_contract_conflicts(&merged, &conflicts_before)?; let changed = merged != policy; Ok(PolicyMergeResult { @@ -300,6 +706,93 @@ pub fn merge_policy( }) } +/// Host and port pairs whose MCP endpoints do not agree on one inspection +/// contract, mapped to the rendered contracts in play. +/// +/// The supervisor selects an endpoint's extended configuration by host and port +/// alone: `endpoint_matches_request` never consults `path`, and the first match +/// wins. Two MCP endpoints on one host and port therefore make the effective +/// strict-tool-name, method-profile, and body-limit contract depend on +/// evaluation order rather than on policy. `merge_endpoint` cannot catch this on +/// its own, because it only compares endpoints that fold together, and a +/// differing path or a separate rule keeps them apart. +fn conflicting_mcp_host_ports(policy: &SandboxPolicy) -> BTreeMap<(String, u32), Vec> { + let mut contracts: BTreeMap<(String, u32), Vec<(EffectiveMcpContract, String)>> = + BTreeMap::new(); + for rule in policy.network_policies.values() { + for endpoint in &rule.endpoints { + // Provider-composed rules are deliberately included: the supervisor + // resolves one contract per host and port regardless of which rule + // contributed the endpoint. + let Some(contract) = effective_mcp_contract(endpoint) else { + continue; + }; + for port in canonical_ports(endpoint) { + contracts + .entry((endpoint.host.to_ascii_lowercase(), port)) + .or_default() + .push((contract, describe_mcp_contract(endpoint))); + } + } + } + + contracts + .into_iter() + .filter_map(|(key, found)| { + let first = found.first()?.0; + if found.iter().all(|(contract, _)| *contract == first) { + return None; + } + let mut rendered: Vec = + found.into_iter().map(|(_, rendered)| rendered).collect(); + rendered.sort(); + rendered.dedup(); + Some((key, rendered)) + }) + .collect() +} + +/// Rejects an operation that introduces an MCP contract conflict. +/// +/// Only conflicts absent from `before` are rejected. A policy that already +/// carries one, for instance through a provider profile composed outside this +/// merge, would otherwise make every later update fail with an error the +/// operation did nothing to cause. +fn ensure_no_new_mcp_contract_conflicts( + merged: &SandboxPolicy, + before: &BTreeMap<(String, u32), Vec>, +) -> Result<(), PolicyMergeError> { + for ((host, port), contracts) in conflicting_mcp_host_ports(merged) { + if before.contains_key(&(host.clone(), port)) { + continue; + } + return Err(PolicyMergeError::ConflictingMcpContractsForEndpoint { + host, + port, + contracts, + }); + } + Ok(()) +} + +fn validate_operation( + operation_index: usize, + operation: &PolicyMergeOp, +) -> Result<(), PolicyMergeError> { + if let PolicyMergeOp::AddRule { rule_name, rule } = operation { + if rule_name.trim().is_empty() { + return Err(PolicyMergeError::MissingRuleNameForAddRule); + } + if rule.endpoints.is_empty() { + return Err(PolicyMergeError::EmptyAddRuleEndpoints { + operation_index, + rule_name: rule_name.clone(), + }); + } + } + Ok(()) +} + pub fn generated_rule_name(host: &str, port: u32) -> String { let sanitized = host .replace(['.', '-'], "_") @@ -311,12 +804,13 @@ pub fn generated_rule_name(host: &str, port: u32) -> String { fn apply_operation( policy: &mut SandboxPolicy, + operation_index: usize, operation: &PolicyMergeOp, warnings: &mut Vec, ) -> Result<(), PolicyMergeError> { match operation { PolicyMergeOp::AddRule { rule_name, rule } => { - add_rule(policy, rule_name, rule, warnings)?; + add_rule(policy, operation_index, rule_name, rule, warnings)?; } PolicyMergeOp::RemoveEndpoint { rule_name, @@ -333,6 +827,7 @@ fn apply_operation( port, deny_rules, } => { + ensure_endpoint_target_is_unambiguous(policy, host, *port)?; let endpoint = find_endpoint_mut(policy, host, *port).ok_or_else(|| { PolicyMergeError::EndpointNotFound { host: host.clone(), @@ -349,6 +844,7 @@ fn apply_operation( append_unique_deny_rules(&mut endpoint.deny_rules, deny_rules); } PolicyMergeOp::AddAllowRules { host, port, rules } => { + ensure_endpoint_target_is_unambiguous(policy, host, *port)?; let endpoint = find_endpoint_mut(policy, host, *port).ok_or_else(|| { PolicyMergeError::EndpointNotFound { host: host.clone(), @@ -363,9 +859,28 @@ fn apply_operation( rule_name, binary_path, } => { + // An empty binary list authorizes every binary, so there is no entry + // to drop and narrowing the rule would mean naming every binary that + // should keep access. Reject instead of reporting a success that + // leaves the target binary authorized. + if policy + .network_policies + .get(rule_name) + .is_some_and(|rule| rule.binaries.is_empty()) + { + return Err(PolicyMergeError::CannotRemoveBinaryFromAnyBinaryScope { + operation_index, + rule_name: rule_name.clone(), + binary_path: binary_path.clone(), + }); + } + let should_remove = if let Some(rule) = policy.network_policies.get_mut(rule_name) { let original_len = rule.binaries.len(); rule.binaries.retain(|binary| binary.path != *binary_path); + // Removing the last named binary deletes the rule rather than + // leaving an empty list behind, which would silently widen the + // rule to every binary. original_len != rule.binaries.len() && rule.binaries.is_empty() } else { false @@ -380,14 +895,11 @@ fn apply_operation( fn add_rule( policy: &mut SandboxPolicy, + operation_index: usize, rule_name: &str, incoming_rule: &NetworkPolicyRule, warnings: &mut Vec, ) -> Result<(), PolicyMergeError> { - if rule_name.trim().is_empty() { - return Err(PolicyMergeError::MissingRuleNameForAddRule); - } - let mut incoming_rule = incoming_rule.clone(); normalize_rule(&mut incoming_rule); if incoming_rule.name.is_empty() { @@ -409,7 +921,8 @@ fn add_rule( // provider rule's binary list. The agent's contribution is kept on its // own rule key, the prover sees the actual narrow proposal, and the // reviewer gets honest signal about what's being added. - let target_key = if policy.network_policies.contains_key(rule_name) { + let requested_key_exists = policy.network_policies.contains_key(rule_name); + let target_key = if requested_key_exists { Some(rule_name.to_string()) } else { let mut keys: Vec<_> = policy.network_policies.keys().cloned().collect(); @@ -426,46 +939,168 @@ fn add_rule( }) }; - if let Some(key) = target_key { - let existing_rule = policy - .network_policies - .get_mut(&key) - .expect("existing rule must be present"); - merge_rules(existing_rule, &incoming_rule, warnings)?; - } else { - policy - .network_policies - .insert(rule_name.to_string(), incoming_rule); + match target_key { + // The operation named this rule, so an inheritance conflict is the + // answer the proposer needs: declare the rule's whole binary and + // endpoint scope, or target a different rule. + Some(key) if requested_key_exists => { + let existing_rule = policy + .network_policies + .get_mut(&key) + .expect("existing rule must be present"); + merge_rules( + existing_rule, + &incoming_rule, + operation_index, + rule_name, + &key, + warnings, + )?; + } + // The overlap fallback chose this rule. Folding is a convenience for + // incremental refinement, not something the operation asked for, so a + // conflict it creates is not the proposer's error to fix: keeping the + // proposal on its requested key authorizes exactly the product the + // operation declared and nothing else. Merging into a copy keeps the + // rejected attempt from leaving partial edits behind. + Some(key) => { + let mut candidate = policy + .network_policies + .get(&key) + .expect("existing rule must be present") + .clone(); + let mut candidate_warnings = Vec::new(); + match merge_rules( + &mut candidate, + &incoming_rule, + operation_index, + rule_name, + &key, + &mut candidate_warnings, + ) { + Ok(()) => { + policy.network_policies.insert(key, candidate); + warnings.extend(candidate_warnings); + } + Err(error) if is_authorization_inheritance_conflict(&error) => { + // Surface the skipped fold. The operator asked for one rule + // and gets two, and the same host and port now appears in + // both, which changes which rule later host-and-port + // operations resolve to. + warnings.push(PolicyMergeWarning::KeptRequestedRuleNameToAvoidWidening { + rule_name: rule_name.to_string(), + overlapping_rule_name: key, + reason: error.to_string(), + }); + policy + .network_policies + .insert(rule_name.to_string(), incoming_rule); + } + Err(error) => return Err(error), + } + } + None => { + policy + .network_policies + .insert(rule_name.to_string(), incoming_rule); + } } Ok(()) } +/// True for the merge errors that exist only because two authorizations share +/// one rule, so moving the incoming authorization to its own rule resolves them. +/// +/// An MCP contract conflict is deliberately excluded. The supervisor establishes +/// one inspection contract per host and port rather than per rule, so two +/// contracts for the same endpoint stay ambiguous in separate rules and the +/// operation has to be rejected wherever it lands. +fn is_authorization_inheritance_conflict(error: &PolicyMergeError) -> bool { + matches!( + error, + PolicyMergeError::NewBinaryWouldInheritAuthorization { .. } + | PolicyMergeError::ExistingBinariesWouldInheritAuthorization { .. } + ) +} + fn merge_rules( existing_rule: &mut NetworkPolicyRule, incoming_rule: &NetworkPolicyRule, + operation_index: usize, + rule_name: &str, + // Key of the rule actually being modified. Differs from `rule_name` when the + // endpoint-overlap fallback folded the operation into another rule, so + // warnings name the rule the operator would have to inspect. + target_rule_name: &str, warnings: &mut Vec, ) -> Result<(), PolicyMergeError> { - append_unique_binaries(&mut existing_rule.binaries, &incoming_rule.binaries); - + // A rule authorizes the Cartesian product of its binaries and endpoints. + // Build the final endpoint set off to the side, then reject any implicit + // grants before publishing either half of that product. + let mut merged_endpoints = existing_rule.endpoints.clone(); + let mut endpoint_warnings = Vec::new(); for incoming_endpoint in &incoming_rule.endpoints { let mut incoming_endpoint = incoming_endpoint.clone(); normalize_endpoint(&mut incoming_endpoint); if let Some(existing_endpoint) = - find_matching_endpoint_mut(&mut existing_rule.endpoints, &incoming_endpoint) + find_matching_endpoint_mut(&mut merged_endpoints, &incoming_endpoint) { - merge_endpoint(existing_endpoint, &incoming_endpoint, warnings)?; + merge_endpoint( + existing_endpoint, + &incoming_endpoint, + operation_index, + &mut endpoint_warnings, + )?; } else { - existing_rule.endpoints.push(incoming_endpoint); + merged_endpoints.push(incoming_endpoint); } } + ensure_authorization_inheritance_is_declared( + existing_rule, + incoming_rule, + &merged_endpoints, + operation_index, + rule_name, + )?; + + existing_rule.endpoints = merged_endpoints; + if existing_rule.binaries.is_empty() { + // The rule already authorizes any binary, so an incoming named list is + // a subset of what is already granted. Appending it would make the list + // non-empty and revoke every binary the operation did not name, turning + // an additive operation into a silent mass revocation. Keep the wider + // scope and say so; narrowing is `RemoveBinary` or a full replacement. + if !incoming_rule.binaries.is_empty() { + warnings.push(PolicyMergeWarning::ExistingAnyBinaryScopeRetained { + rule_name: target_rule_name.to_string(), + incoming: incoming_rule + .binaries + .iter() + .map(|binary| binary.path.clone()) + .collect(), + }); + } + } else if incoming_rule.binaries.is_empty() { + // An incoming any-binary scope replaces the restricted scope instead of + // appending to it. Appending an empty list would leave the existing + // binaries in place, so the operation would report success while never + // authorizing the scope it asked for and coverage would never converge. + // The inheritance check above already required this operation to + // declare every merged endpoint before the widening is allowed. + existing_rule.binaries.clear(); + } else { + append_unique_binaries(&mut existing_rule.binaries, &incoming_rule.binaries); + } + warnings.extend(endpoint_warnings); Ok(()) } fn merge_endpoint( existing: &mut NetworkEndpoint, incoming: &NetworkEndpoint, + operation_index: usize, warnings: &mut Vec, ) -> Result<(), PolicyMergeError> { let host = if existing.host.is_empty() { @@ -473,12 +1108,17 @@ fn merge_endpoint( } else { existing.host.clone() }; - let port = canonical_ports(existing) + let existing_ports = canonical_ports(existing); + let incoming_ports = canonical_ports(incoming); + let port = existing_ports .into_iter() - .next() - .or_else(|| canonical_ports(incoming).into_iter().next()) + .find(|port| incoming_ports.contains(port)) + .or_else(|| incoming_ports.first().copied()) .unwrap_or(0); + let promotes_l4_to_mcp = promotes_l4_endpoint_to_mcp(existing, incoming); + ensure_mcp_contract_compatible(existing, incoming, operation_index, &host, port)?; + if existing.host.is_empty() { existing.host.clone_from(&incoming.host); } @@ -499,6 +1139,10 @@ fn merge_endpoint( }, warnings, ); + if promotes_l4_to_mcp { + existing.mcp.clone_from(&incoming.mcp); + existing.json_rpc_max_body_bytes = incoming.json_rpc_max_body_bytes; + } let existing_enforcement = existing.enforcement.clone(); merge_string_field( &mut existing.enforcement, @@ -563,42 +1207,346 @@ fn merge_endpoint( Ok(()) } -fn merge_string_field( - existing: &mut String, - incoming: &str, - warning: PolicyMergeWarning, - warnings: &mut Vec, -) { - if incoming.is_empty() { - return; - } - if existing.is_empty() { - *existing = incoming.to_string(); - } else if *existing != incoming { - warnings.push(warning); +fn ensure_mcp_contract_compatible( + existing: &NetworkEndpoint, + incoming: &NetworkEndpoint, + operation_index: usize, + host: &str, + port: u32, +) -> Result<(), PolicyMergeError> { + if promotes_l4_endpoint_to_mcp(existing, incoming) || mcp_contracts_match(existing, incoming) { + return Ok(()); } + + Err(PolicyMergeError::McpContractConflict { + operation_index, + host: host.to_string(), + port, + existing: describe_mcp_contract(existing), + incoming: describe_mcp_contract(incoming), + }) } -fn merge_endpoint_ports(existing: &mut NetworkEndpoint, incoming: &NetworkEndpoint) { - let mut ports = canonical_ports(existing); - for port in canonical_ports(incoming) { - if !ports.contains(&port) { - ports.push(port); - } - } - ports.sort_unstable(); - ports.dedup(); - existing.port = ports.first().copied().unwrap_or(0); - existing.ports = ports; +/// Renders the values `mcp_contracts_match` compares, so the reported conflict +/// names the fields that have to agree. The effective contract is rendered +/// rather than the raw options because an omitted option and its default are +/// the same contract. +fn describe_mcp_contract(endpoint: &NetworkEndpoint) -> String { + effective_mcp_contract(endpoint).map_or_else( + || format!("non-mcp(protocol='{}')", endpoint.protocol), + |contract| { + format!( + "mcp(strict_tool_names={}, allow_all_known_mcp_methods={}, max_body_bytes={})", + contract.strict_tool_names, + contract.allow_all_known_mcp_methods, + contract.max_body_bytes + ) + }, + ) } -fn rules_share_endpoint( +fn promotes_l4_endpoint_to_mcp(existing: &NetworkEndpoint, incoming: &NetworkEndpoint) -> bool { + // Only an endpoint without an established inspection contract can adopt + // the complete incoming MCP contract instead of combining two contracts. + existing.protocol.is_empty() + && existing.mcp.is_none() + && existing.json_rpc_max_body_bytes == 0 + && effective_mcp_contract(incoming).is_some() +} + +fn ensure_authorization_inheritance_is_declared( existing_rule: &NetworkPolicyRule, incoming_rule: &NetworkPolicyRule, -) -> bool { - incoming_rule.endpoints.iter().any(|incoming_endpoint| { - existing_rule - .endpoints + merged_endpoints: &[NetworkEndpoint], + operation_index: usize, + rule_name: &str, +) -> Result<(), PolicyMergeError> { + let existing_binary_paths: HashSet<&str> = existing_rule + .binaries + .iter() + .map(|binary| binary.path.as_str()) + .collect(); + // Binary scope the operation adds to the rule. An empty incoming list means + // any binary, which introduces every binary outside the existing list, so it + // widens this side of the Cartesian product just as a new concrete path does + // and has to declare the endpoints it will reach. An existing empty list + // already authorizes any binary, so nothing incoming can widen it further. + let new_binary_scope = if existing_rule.binaries.is_empty() { + None + } else if incoming_rule.binaries.is_empty() { + Some(ANY_BINARY_SCOPE.to_string()) + } else { + incoming_rule + .binaries + .iter() + .find(|binary| !existing_binary_paths.contains(binary.path.as_str())) + .map(|binary| format!("binary '{}'", binary.path)) + }; + let undeclared_binaries = undeclared_existing_binaries(existing_rule, incoming_rule); + + for endpoint in merged_endpoints { + let units = authorization_ports(endpoint); + + if let Some(binary_scope) = &new_binary_scope { + // Adoption depends on the declaration and this endpoint, not on the + // port, so it is resolved once per endpoint rather than per port. + let declarations: Vec = incoming_rule + .endpoints + .iter() + .map(|declared| adopt_unset_retained_fields(declared, endpoint)) + .collect(); + // The operation may declare one endpoint's ports across several + // incoming endpoints, so each port is checked against every + // declaration rather than against a single one. + let undeclared_ports: Vec> = units + .iter() + .copied() + .filter(|port| !operation_declares(&declarations, endpoint, *port)) + .collect(); + if !undeclared_ports.is_empty() { + return Err(PolicyMergeError::NewBinaryWouldInheritAuthorization { + operation_index, + rule_name: rule_name.to_string(), + binary_scope: binary_scope.clone(), + host: endpoint.host.clone(), + ports: undeclared_ports.into_iter().flatten().collect(), + }); + } + } + + let changed_ports: Vec> = units + .iter() + .copied() + .filter(|port| { + !existing_rule + .endpoints + .iter() + .any(|existing| authorization_unit_unchanged(existing, endpoint, *port)) + }) + .collect(); + + // Every changed port must be named by the operation, whether or not the + // rule's binary scope is fully declared. Widened fields merge into the + // endpoint as a whole, so a change declared for one port of a + // multi-port endpoint reaches that endpoint's other ports too. Checking + // this only alongside undeclared binaries would let an operation that + // lists every existing binary widen an undeclared port. + let undeclared_changed_ports: Vec> = changed_ports + .iter() + .copied() + .filter(|port| !operation_names_port(incoming_rule, endpoint, *port)) + .collect(); + if !undeclared_changed_ports.is_empty() { + return Err(PolicyMergeError::UndeclaredPortWouldChange { + operation_index, + rule_name: rule_name.to_string(), + host: endpoint.host.clone(), + ports: undeclared_changed_ports.into_iter().flatten().collect(), + }); + } + + if !changed_ports.is_empty() && !undeclared_binaries.is_empty() { + return Err( + PolicyMergeError::ExistingBinariesWouldInheritAuthorization { + operation_index, + rule_name: rule_name.to_string(), + host: endpoint.host.clone(), + ports: changed_ports.into_iter().flatten().collect(), + undeclared_binaries, + }, + ); + } + } + + Ok(()) +} + +/// True when the operation names `port` on an endpoint matching `merged`'s host +/// and path. +/// +/// This asks only whether the operation addressed the port, not whether it +/// restated the endpoint's authorization. Pre-existing authorization on a port +/// the operation names is not something the operation granted, but a widened +/// field landing on a port the operation never named is. +fn operation_names_port( + incoming_rule: &NetworkPolicyRule, + merged: &NetworkEndpoint, + port: Option, +) -> bool { + incoming_rule.endpoints.iter().any(|declared| { + declared.host.eq_ignore_ascii_case(&merged.host) + && declared.path == merged.path + && port.is_none_or(|port| canonical_ports(declared).contains(&port)) + }) +} + +/// True when the operation itself declares the authorization `merged` grants at +/// `port`, anywhere in `declarations`. +/// +/// `declarations` must already have been through `adopt_unset_retained_fields` +/// against `merged`. +fn operation_declares( + declarations: &[NetworkEndpoint], + merged: &NetworkEndpoint, + port: Option, +) -> bool { + declarations + .iter() + .any(|declared| endpoint_authorization_covers_port(declared, merged, port)) +} + +/// Fills a declaration's unset carry-over fields from the endpoint it is being +/// judged against. +/// +/// A carry-over field is one the merge never widens: it either retains the +/// existing value (`protocol`, `tls`, `enforcement`, `access`) or ignores the +/// incoming value entirely (the signing and GraphQL fields). Either way the new +/// binary receives whatever the endpoint already carried, and the operation +/// cannot change it, so a declaration that leaves the field unset expresses no +/// opinion rather than a request for the default. Comparing the unset value +/// against the merged one would reject a complete declaration over a field the +/// operation never tried to change, and for `enforcement` it would reject +/// specifically because the binary is about to receive the stricter mode. +/// +/// Every carry-over field has to be listed here. Missing one costs a false +/// rejection of a complete declaration, which is why adoption is written as an +/// explicit allow-list rather than by copying `merged` and restoring the widened +/// fields: forgetting a field in that direction would silently satisfy the +/// declaration instead. +/// +/// This is the declaration direction only. Coverage still exact-matches a value +/// the proposal actually set, because there the question is whether the +/// proposer's own request took effect. +fn adopt_unset_retained_fields( + declared: &NetworkEndpoint, + merged: &NetworkEndpoint, +) -> NetworkEndpoint { + let mut adopted = declared.clone(); + if adopted.protocol.is_empty() { + adopted.protocol.clone_from(&merged.protocol); + } + if adopted.tls.is_empty() { + adopted.tls.clone_from(&merged.tls); + } + if adopted.enforcement.is_empty() { + adopted.enforcement.clone_from(&merged.enforcement); + } + // `merge_endpoint` only touches `access` when the incoming endpoint carries + // an access preset or explicit rules, so an endpoint declaring neither keeps + // whatever preset is already loaded. + if adopted.access.is_empty() && adopted.rules.is_empty() { + adopted.access.clone_from(&merged.access); + } + if adopted.persisted_queries.is_empty() { + adopted + .persisted_queries + .clone_from(&merged.persisted_queries); + } + if adopted.graphql_persisted_queries.is_empty() { + adopted + .graphql_persisted_queries + .clone_from(&merged.graphql_persisted_queries); + } + if adopted.graphql_max_body_bytes == 0 { + adopted.graphql_max_body_bytes = merged.graphql_max_body_bytes; + } + if adopted.credential_signing.is_empty() { + adopted + .credential_signing + .clone_from(&merged.credential_signing); + } + if adopted.signing_service.is_empty() { + adopted.signing_service.clone_from(&merged.signing_service); + } + if adopted.signing_region.is_empty() { + adopted.signing_region.clone_from(&merged.signing_region); + } + if adopted.json_rpc_max_body_bytes == 0 { + adopted.json_rpc_max_body_bytes = merged.json_rpc_max_body_bytes; + } + adopted +} + +/// True when `existing` already authorizes exactly what `merged` authorizes at +/// `port`. Equivalence rather than coverage: an existing endpoint authorizing +/// strictly less has changed, and the rule's other binaries would inherit the +/// difference. +fn authorization_unit_unchanged( + existing: &NetworkEndpoint, + merged: &NetworkEndpoint, + port: Option, +) -> bool { + endpoint_authorization_covers_port(existing, merged, port) + && endpoint_authorization_covers_port(merged, existing, port) +} + +/// Existing binary scope the incoming rule failed to declare. Empty means the +/// operation covers the whole existing scope and no binary can inherit a new +/// endpoint implicitly. +/// +/// An empty binary list means any binary: an incoming any-binary scope covers +/// every existing binary, while a specific incoming list can never claim an +/// existing any-binary scope. +fn undeclared_existing_binaries( + existing_rule: &NetworkPolicyRule, + incoming_rule: &NetworkPolicyRule, +) -> Vec { + if incoming_rule.binaries.is_empty() { + return Vec::new(); + } + if existing_rule.binaries.is_empty() { + return vec!["the existing any-binary scope".to_string()]; + } + + existing_rule + .binaries + .iter() + .filter(|existing| { + !incoming_rule + .binaries + .iter() + .any(|incoming| incoming.path == existing.path) + }) + .map(|existing| existing.path.clone()) + .collect() +} + +fn merge_string_field( + existing: &mut String, + incoming: &str, + warning: PolicyMergeWarning, + warnings: &mut Vec, +) { + if incoming.is_empty() { + return; + } + if existing.is_empty() { + *existing = incoming.to_string(); + } else if *existing != incoming { + warnings.push(warning); + } +} + +fn merge_endpoint_ports(existing: &mut NetworkEndpoint, incoming: &NetworkEndpoint) { + let mut ports = canonical_ports(existing); + for port in canonical_ports(incoming) { + if !ports.contains(&port) { + ports.push(port); + } + } + ports.sort_unstable(); + ports.dedup(); + existing.port = ports.first().copied().unwrap_or(0); + existing.ports = ports; +} + +fn rules_share_endpoint( + existing_rule: &NetworkPolicyRule, + incoming_rule: &NetworkPolicyRule, +) -> bool { + incoming_rule.endpoints.iter().any(|incoming_endpoint| { + existing_rule + .endpoints .iter() .any(|existing_endpoint| endpoints_overlap(existing_endpoint, incoming_endpoint)) }) @@ -636,6 +1584,53 @@ fn find_matching_endpoint_mut<'a>( .find(|endpoint| endpoints_overlap(endpoint, target)) } +/// Every endpoint `host:port` resolves to, rendered with its owning rule. +/// +/// `endpoint_matches_host_port` deliberately ignores `path`, and +/// `endpoints_overlap` treats a different path as a different endpoint, so one +/// rule can own several matching endpoints. Counting rules alone would miss +/// that, so this counts endpoints. +fn matching_endpoint_targets(policy: &SandboxPolicy, host: &str, port: u32) -> Vec { + let mut targets: Vec = policy + .network_policies + .iter() + .filter(|(key, _)| !is_provider_rule_name(key)) + .flat_map(|(key, rule)| { + rule.endpoints + .iter() + .filter(|endpoint| endpoint_matches_host_port(endpoint, host, port)) + .map(move |endpoint| { + if endpoint.path.is_empty() { + key.clone() + } else { + format!("{key} (path '{}')", endpoint.path) + } + }) + }) + .collect(); + targets.sort(); + targets +} + +/// Fails closed when `host:port` resolves to more than one endpoint, because +/// appending an L7 rule widens authorization for every binary on whichever +/// endpoint is picked, and the operation cannot say which one it means. +fn ensure_endpoint_target_is_unambiguous( + policy: &SandboxPolicy, + host: &str, + port: u32, +) -> Result<(), PolicyMergeError> { + let targets = matching_endpoint_targets(policy, host, port); + if targets.len() > 1 { + return Err(PolicyMergeError::AmbiguousEndpointRule { + host: host.to_string(), + port, + targets, + }); + } + Ok(()) +} + fn find_endpoint_mut<'a>( policy: &'a mut SandboxPolicy, host: &str, @@ -917,12 +1912,13 @@ mod tests { use std::collections::HashMap; use super::{ - PolicyMergeError, PolicyMergeOp, PolicyMergeWarning, generated_rule_name, merge_policy, - policy_covers_rule, + ANY_BINARY_SCOPE, DEFAULT_JSON_RPC_MAX_BODY_BYTES, PolicyMergeError, PolicyMergeOp, + PolicyMergeWarning, generated_rule_name, merge_policy, policy_covers_rule, }; use crate::restrictive_default_policy; use openshell_core::proto::{ - L7Allow, L7DenyRule, L7Rule, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, + L7Allow, L7DenyRule, L7QueryMatcher, L7Rule, McpOptions, NetworkBinary, NetworkEndpoint, + NetworkPolicyRule, SandboxPolicy, }; fn endpoint(host: &str, port: u32) -> NetworkEndpoint { @@ -969,6 +1965,70 @@ mod tests { } } + fn binary(path: &str) -> NetworkBinary { + NetworkBinary { + path: path.to_string(), + ..Default::default() + } + } + + fn mcp_tool_rule(tool: &str) -> L7Rule { + L7Rule { + allow: Some(L7Allow { + method: "tools/call".to_string(), + params: HashMap::from([( + "name".to_string(), + L7QueryMatcher { + glob: tool.to_string(), + any: Vec::new(), + }, + )]), + ..Default::default() + }), + } + } + + fn mcp_endpoint( + host: &str, + ports: &[u32], + strict_tool_names: Option, + allow_all_known_mcp_methods: Option, + max_body_bytes: u32, + rules: Vec, + ) -> NetworkEndpoint { + NetworkEndpoint { + host: host.to_string(), + port: ports.first().copied().unwrap_or_default(), + ports: ports.to_vec(), + protocol: "mcp".to_string(), + rules, + json_rpc_max_body_bytes: max_body_bytes, + mcp: Some(McpOptions { + strict_tool_names, + allow_all_known_mcp_methods, + }), + ..Default::default() + } + } + + fn rule_with_authorizations( + name: &str, + endpoints: Vec, + binaries: &[&str], + ) -> NetworkPolicyRule { + NetworkPolicyRule { + name: name.to_string(), + endpoints, + binaries: binaries.iter().map(|path| binary(path)).collect(), + } + } + + fn policy_with_rule(rule_name: &str, rule: NetworkPolicyRule) -> SandboxPolicy { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert(rule_name.to_string(), rule); + policy + } + #[test] fn generated_rule_name_sanitizes_host() { assert_eq!( @@ -978,240 +2038,959 @@ mod tests { } #[test] - fn add_rule_merges_l7_fields_into_existing_endpoint() { - 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![NetworkBinary { - path: "/usr/bin/curl".to_string(), - ..Default::default() - }], - }, - ); - - let incoming = NetworkPolicyRule { - name: "incoming".to_string(), - endpoints: vec![NetworkEndpoint { - host: "api.github.com".to_string(), - port: 443, - ports: vec![443], - protocol: "rest".to_string(), - enforcement: "enforce".to_string(), - rules: vec![rest_rule("GET", "/repos/**")], - ..Default::default() - }], - binaries: vec![NetworkBinary { - path: "/usr/bin/gh".to_string(), - ..Default::default() - }], - }; - - let result = merge_policy( - policy, + fn add_rule_rejects_empty_endpoints_at_the_library_boundary() { + let error = merge_policy( + restrictive_default_policy(), &[PolicyMergeOp::AddRule { - rule_name: "allow_api_github_com_443".to_string(), - rule: incoming, + rule_name: "empty".to_string(), + rule: rule_with_authorizations("empty", Vec::new(), &["/usr/bin/client"]), }], ) - .expect("merge should succeed"); + .expect_err("an AddRule without authorization endpoints must fail"); - let rule = &result.policy.network_policies["existing"]; - let endpoint = &rule.endpoints[0]; - assert_eq!(endpoint.protocol, "rest"); - assert_eq!(endpoint.enforcement, "enforce"); - assert_eq!(endpoint.rules.len(), 1); - assert_eq!(rule.binaries.len(), 2); + assert_eq!( + error, + PolicyMergeError::EmptyAddRuleEndpoints { + operation_index: 0, + rule_name: "empty".to_string(), + } + ); } #[test] - fn add_rule_user_binary_clears_advisor_marker_for_same_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")], + fn merge_reports_the_first_failing_operation_in_request_order() { + let operations = [ + PolicyMergeOp::AddAllowRules { + host: "missing.example.com".to_string(), + port: 443, + rules: vec![rest_rule("GET", "/")], }, + PolicyMergeOp::AddRule { + rule_name: "empty".to_string(), + rule: NetworkPolicyRule::default(), + }, + ]; + + assert_eq!( + merge_policy(restrictive_default_policy(), &operations), + Err(PolicyMergeError::EndpointNotFound { + host: "missing.example.com".to_string(), + port: 443, + }) ); + } - let incoming = NetworkPolicyRule { - name: "incoming".to_string(), - endpoints: vec![endpoint("api.github.com", 443)], - binaries: vec![NetworkBinary { - path: "/usr/bin/curl".to_string(), - ..Default::default() - }], + #[test] + fn new_binary_cannot_inherit_an_undeclared_existing_rest_endpoint() { + let existing_endpoint = NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/admin")], + ..endpoint("admin.example.com", 443) }; + let existing = + rule_with_authorizations("existing", vec![existing_endpoint], &["/usr/bin/trusted"]); + let incoming = rule_with_authorizations( + "existing", + vec![endpoint("public.example.com", 443)], + &["/usr/bin/untrusted"], + ); - let result = merge_policy( - policy, + let error = merge_policy( + policy_with_rule("existing", existing), &[PolicyMergeOp::AddRule { rule_name: "existing".to_string(), rule: incoming, }], ) - .expect("merge should succeed"); + .expect_err("the new binary did not declare the existing REST endpoint"); - let rule = &result.policy.network_policies["existing"]; - assert_eq!(rule.binaries.len(), 1); - #[allow(deprecated)] - { - assert!(!rule.binaries[0].harness); - } + assert!(matches!( + error, + PolicyMergeError::NewBinaryWouldInheritAuthorization { + operation_index: 0, + binary_scope, + host, + ports, + .. + } if binary_scope == "binary '/usr/bin/untrusted'" + && host == "admin.example.com" + && ports == vec![443] + )); } #[test] - fn add_rule_duplicate_binaries_prefer_user_declared_marker() { - let incoming = NetworkPolicyRule { - name: "incoming".to_string(), - endpoints: vec![endpoint("api.github.com", 443)], - binaries: vec![ - advisor_binary("/usr/bin/curl"), - NetworkBinary { - path: "/usr/bin/curl".to_string(), - ..Default::default() - }, - ], - }; + fn new_binary_cannot_inherit_an_undeclared_existing_mcp_endpoint() { + let existing = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("existing-tool")], + )], + &["/usr/bin/trusted"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![endpoint("unrelated.example.com", 443)], + &["/usr/bin/untrusted"], + ); - let result = merge_policy( - restrictive_default_policy(), + let error = merge_policy( + policy_with_rule("existing", existing), &[PolicyMergeOp::AddRule { - rule_name: "github".to_string(), + rule_name: "existing".to_string(), rule: incoming, }], ) - .expect("merge should succeed"); + .expect_err("the new binary did not declare the existing MCP endpoint"); - let rule = &result.policy.network_policies["github"]; - assert_eq!(rule.binaries.len(), 1); - #[allow(deprecated)] - { - assert!(!rule.binaries[0].harness); - } - } + assert!(matches!( + error, + PolicyMergeError::NewBinaryWouldInheritAuthorization { + operation_index: 0, + binary_scope, + host, + .. + } if binary_scope == "binary '/usr/bin/untrusted'" && host == "mcp.example.com" + )); + } #[test] - fn add_rule_preserves_advisor_endpoint_marker_when_binary_is_deduped() { - let mut policy = restrictive_default_policy(); - policy.network_policies.insert( - "app-api".to_string(), - NetworkPolicyRule { - name: "app-api".to_string(), - endpoints: vec![endpoint("api.example.com", 443)], - binaries: vec![NetworkBinary { - path: "/usr/bin/python".to_string(), - ..Default::default() - }], - }, + fn new_binary_must_declare_every_existing_mcp_endpoint() { + let endpoint_a = mcp_endpoint( + "a.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("a")], + ); + let endpoint_b = mcp_endpoint( + "b.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("b")], ); + let existing = rule_with_authorizations( + "existing", + vec![endpoint_a.clone(), endpoint_b], + &["/usr/bin/trusted"], + ); + let incoming = + rule_with_authorizations("existing", vec![endpoint_a], &["/usr/bin/untrusted"]); - let incoming = NetworkPolicyRule { - name: "app-api".to_string(), - endpoints: vec![NetworkEndpoint { - host: "internal-admin.local".to_string(), - port: 443, - ports: vec![443], - advisor_proposed: true, - ..Default::default() + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, }], - binaries: vec![advisor_binary("/usr/bin/python")], + ) + .expect_err("declaring only one endpoint must not grant the second endpoint"); + + assert!(matches!( + error, + PolicyMergeError::NewBinaryWouldInheritAuthorization { host, .. } + if host == "b.example.com" + )); + } + + #[test] + fn existing_binary_scope_must_be_declared_for_a_new_mcp_endpoint() { + let existing = rule_with_authorizations( + "existing", + vec![endpoint("rest.example.com", 443)], + &["/usr/bin/first", "/usr/bin/second"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("new-tool")], + )], + &["/usr/bin/first"], + ); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect_err("the undeclared second binary would inherit the new MCP endpoint"); + + assert!(matches!( + error, + PolicyMergeError::ExistingBinariesWouldInheritAuthorization { host, .. } + if host == "mcp.example.com" + )); + } + + #[test] + fn existing_any_binary_scope_requires_an_incoming_any_binary_declaration() { + let existing = + rule_with_authorizations("existing", vec![endpoint("rest.example.com", 443)], &[]); + let incoming_endpoint = mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("new-tool")], + ); + let specific_incoming = rule_with_authorizations( + "existing", + vec![incoming_endpoint.clone()], + &["/usr/bin/client"], + ); + + assert!(matches!( + merge_policy( + policy_with_rule("existing", existing.clone()), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: specific_incoming, + }] + ), + Err(PolicyMergeError::ExistingBinariesWouldInheritAuthorization { .. }) + )); + + let any_binary_incoming = + rule_with_authorizations("existing", vec![incoming_endpoint], &[]); + assert!( + merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: any_binary_incoming, + }] + ) + .is_ok(), + "an explicit any-binary proposal covers the existing any-binary scope" + ); + } + + #[test] + fn l4_endpoint_promotion_to_mcp_requires_the_complete_existing_binary_scope() { + let existing = rule_with_authorizations( + "existing", + vec![endpoint("mcp.example.com", 443)], + &["/usr/bin/first", "/usr/bin/second"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("new-tool")], + )], + &["/usr/bin/first"], + ); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect_err("the second binary did not declare the MCP promotion"); + + assert!(matches!( + error, + PolicyMergeError::ExistingBinariesWouldInheritAuthorization { + operation_index: 0, + host, + ports, + .. + } if host == "mcp.example.com" && ports == vec![443] + )); + } + + #[test] + fn l4_endpoint_promotion_to_mcp_preserves_the_declared_contract() { + let existing = rule_with_authorizations( + "existing", + vec![endpoint("mcp.example.com", 443)], + &["/usr/bin/first", "/usr/bin/second"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + Some(false), + Some(true), + 128 * 1024, + vec![mcp_tool_rule("new-tool")], + )], + &["/usr/bin/first", "/usr/bin/second"], + ); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect("the complete binary scope explicitly accepts the MCP promotion"); + + let promoted = &result.policy.network_policies["existing"].endpoints[0]; + assert_eq!(promoted.protocol, "mcp"); + assert_eq!(promoted.json_rpc_max_body_bytes, 128 * 1024); + assert_eq!( + promoted.mcp, + Some(McpOptions { + strict_tool_names: Some(false), + allow_all_known_mcp_methods: Some(true), + }) + ); + assert_eq!(promoted.rules, vec![mcp_tool_rule("new-tool")]); + } + + #[test] + fn established_rest_endpoint_cannot_be_reinterpreted_as_mcp() { + let existing_endpoint = NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/")], + ..endpoint("api.example.com", 443) }; + let existing = + rule_with_authorizations("existing", vec![existing_endpoint], &["/usr/bin/client"]); + let incoming = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "api.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("tool")], + )], + &["/usr/bin/client"], + ); + + assert!(matches!( + merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }] + ), + Err(PolicyMergeError::McpContractConflict { + existing, incoming, .. + }) if existing == "non-mcp(protocol='rest')" && incoming.starts_with("mcp(") + )); + } + + #[test] + fn mcp_overlap_uses_effective_defaults_and_rejects_body_limit_changes() { + let existing_endpoint = mcp_endpoint( + "mcp.example.com", + &[443, 8443], + None, + None, + 0, + vec![mcp_tool_rule("tool")], + ); + let explicit_defaults = mcp_endpoint( + "mcp.example.com", + &[8443], + Some(true), + Some(false), + DEFAULT_JSON_RPC_MAX_BODY_BYTES, + vec![mcp_tool_rule("tool")], + ); + let existing = + rule_with_authorizations("existing", vec![existing_endpoint], &["/usr/bin/client"]); + let equivalent = rule_with_authorizations( + "incoming", + vec![explicit_defaults.clone()], + &["/usr/bin/client"], + ); + assert!( + merge_policy( + policy_with_rule("existing", existing.clone()), + &[PolicyMergeOp::AddRule { + rule_name: "incoming".to_string(), + rule: equivalent, + }] + ) + .is_ok(), + "omitted MCP booleans and body limit must equal their runtime defaults" + ); + + let mut different_body_limit = explicit_defaults; + different_body_limit.json_rpc_max_body_bytes = 128 * 1024; + let conflicting = + rule_with_authorizations("incoming", vec![different_body_limit], &["/usr/bin/client"]); + assert!(matches!( + merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "incoming".to_string(), + rule: conflicting, + }] + ), + // The existing endpoint omitted the limit, so the conflict reports + // its effective default rather than the raw zero. + Err(PolicyMergeError::McpContractConflict { + port: 8443, + existing, + incoming, + .. + }) if existing.contains("max_body_bytes=65536") + && incoming.contains("max_body_bytes=131072") + )); + } + + #[test] + fn non_overlapping_mcp_endpoints_may_use_different_contracts() { + let existing = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "first.example.com", + &[443, 8443], + None, + None, + 0, + vec![mcp_tool_rule("first-tool")], + )], + &["/usr/bin/client"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "second.example.com", + &[443], + Some(false), + Some(true), + 128 * 1024, + vec![mcp_tool_rule("second-tool")], + )], + &["/usr/bin/client"], + ); let result = merge_policy( - policy, + policy_with_rule("existing", existing), &[PolicyMergeOp::AddRule { - rule_name: "app-api".to_string(), + rule_name: "existing".to_string(), rule: incoming, }], ) - .expect("merge should succeed"); + .expect("contract differences on disjoint endpoints are independent"); - 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!( + result.policy.network_policies["existing"].endpoints.len(), + 2 + ); + } + + #[test] + fn policy_coverage_checks_full_mcp_matchers_ports_contract_and_defaults() { + let loaded_endpoint = mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 0, + vec![mcp_tool_rule("loaded-tool")], + ); + let loaded = policy_with_rule( + "loaded", + rule_with_authorizations( + "loaded", + vec![loaded_endpoint.clone()], + &["/usr/bin/client"], + ), + ); + + let wrong_tool = rule_with_authorizations( + "proposed", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + Some(true), + Some(false), + DEFAULT_JSON_RPC_MAX_BODY_BYTES, + vec![mcp_tool_rule("different-tool")], + )], + &["/usr/bin/client"], + ); + assert!(!policy_covers_rule(&loaded, &wrong_tool)); + + let extra_port = rule_with_authorizations( + "proposed", + vec![mcp_endpoint( + "mcp.example.com", + &[443, 8443], + None, + None, + 0, + vec![mcp_tool_rule("loaded-tool")], + )], + &["/usr/bin/client"], + ); + assert!(!policy_covers_rule(&loaded, &extra_port)); + + let equivalent_defaults = rule_with_authorizations( + "proposed", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + Some(true), + Some(false), + DEFAULT_JSON_RPC_MAX_BODY_BYTES, + vec![mcp_tool_rule("loaded-tool")], + )], + &["/usr/bin/client"], + ); + assert!(policy_covers_rule(&loaded, &equivalent_defaults)); + + let different_body = rule_with_authorizations( + "proposed", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 128 * 1024, + vec![mcp_tool_rule("loaded-tool")], + )], + &["/usr/bin/client"], + ); + assert!(!policy_covers_rule(&loaded, &different_body)); + + let mut explicit_defaults = loaded_endpoint; + explicit_defaults.tls = "passthrough".to_string(); + explicit_defaults.enforcement = "audit".to_string(); + let runtime_defaults = rule_with_authorizations( + "proposed", + vec![explicit_defaults.clone()], + &["/usr/bin/client"], + ); + assert!(policy_covers_rule(&loaded, &runtime_defaults)); + + explicit_defaults.tls = "terminate".to_string(); + let legacy_terminate = rule_with_authorizations( + "proposed", + vec![explicit_defaults.clone()], + &["/usr/bin/client"], + ); + assert!(policy_covers_rule(&loaded, &legacy_terminate)); + + explicit_defaults.tls = "skip".to_string(); + let skip_tls = rule_with_authorizations( + "proposed", + vec![explicit_defaults.clone()], + &["/usr/bin/client"], + ); + assert!(!policy_covers_rule(&loaded, &skip_tls)); + + explicit_defaults.tls.clear(); + explicit_defaults.enforcement = "enforce".to_string(); + let different_runtime_scalars = + rule_with_authorizations("proposed", vec![explicit_defaults], &["/usr/bin/client"]); + assert!(!policy_covers_rule(&loaded, &different_runtime_scalars)); + } + + /// `policy_covers_rule` answers "is my rule in effect?" for the sandbox's + /// `/wait` long-poll, so anything `merge_policy` accepts must read back as + /// covered once loaded. Otherwise the poll spins to its deadline and the + /// agent is told its approved rule never landed. + #[test] + fn merged_policy_covers_the_rule_it_merged() { + // Every field `merge_endpoint` widens rather than replaces, set on the + // existing endpoint and absent from the proposal. + let existing_endpoint = NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/health")], + deny_rules: vec![L7DenyRule { + method: "DELETE".to_string(), + path: "/*".to_string(), + ..Default::default() + }], + allowed_ips: vec!["10.0.0.1".to_string()], + allow_encoded_slash: true, + websocket_credential_rewrite: true, + request_body_credential_rewrite: true, + advisor_proposed: true, + ..endpoint("api.example.com", 443) + }; + let proposed = rule_with_authorizations( + "api", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/users")], + ..endpoint("api.example.com", 443) + }], + &["/usr/bin/client"], + ); + + let merged = merge_policy( + policy_with_rule( + "api", + rule_with_authorizations("api", vec![existing_endpoint], &["/usr/bin/client"]), + ), + &[PolicyMergeOp::AddRule { + rule_name: "api".to_string(), + rule: proposed.clone(), + }], + ) + .expect("merge must accept a proposal that declares the full binary scope"); + + assert!(policy_covers_rule(&merged.policy, &proposed)); + } + + #[test] + fn policy_coverage_requires_proposed_denies_but_allows_loaded_only_denies() { + let deny = |path: &str| L7DenyRule { + method: "GET".to_string(), + path: path.to_string(), + ..Default::default() + }; + let proposed_endpoint = NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/users")], + deny_rules: vec![deny("/users/secret")], + ..endpoint("api.example.com", 443) + }; + let proposed = rule_with_authorizations( + "proposed", + vec![proposed_endpoint.clone()], + &["/usr/bin/client"], + ); + let cover_with = |deny_rules: Vec| { + let loaded_endpoint = NetworkEndpoint { + deny_rules, + ..proposed_endpoint.clone() + }; + policy_covers_rule( + &policy_with_rule( + "loaded", + rule_with_authorizations("loaded", vec![loaded_endpoint], &["/usr/bin/client"]), + ), + &proposed, + ) + }; + + assert!( + !cover_with(Vec::new()), + "a proposed deny that is not loaded means the proposal is not in effect" + ); + assert!( + cover_with(vec![deny("/users/secret")]), + "the proposed deny alone is coverage" + ); + assert!( + cover_with(vec![deny("/users/secret"), deny("/admin")]), + "a deny carried by the loaded endpoint from an earlier merge must not block coverage" + ); + } + + #[test] + fn policy_coverage_requires_endpoint_advisor_provenance_to_be_loaded() { + let loaded_endpoint = endpoint("api.example.com", 443); + let mut proposed_endpoint = loaded_endpoint.clone(); + proposed_endpoint.advisor_proposed = true; + let loaded = policy_with_rule( + "loaded", + rule_with_authorizations("loaded", vec![loaded_endpoint], &["/usr/bin/client"]), + ); + let proposed = + rule_with_authorizations("proposed", vec![proposed_endpoint], &["/usr/bin/client"]); + + assert!(!policy_covers_rule(&loaded, &proposed)); + } + + #[test] + fn policy_coverage_checks_every_binary_endpoint_pair_across_rule_union() { + let proposed = rule_with_authorizations( + "proposed", + vec![ + endpoint("a.example.com", 443), + endpoint("b.example.com", 443), + ], + &["/usr/bin/first", "/usr/bin/second"], + ); + let mut loaded = restrictive_default_policy(); + for (name, host, binary_path) in [ + ("a-first", "a.example.com", "/usr/bin/first"), + ("a-second", "a.example.com", "/usr/bin/second"), + ("b-first", "b.example.com", "/usr/bin/first"), + ] { + loaded.network_policies.insert( + name.to_string(), + rule_with_authorizations(name, vec![endpoint(host, 443)], &[binary_path]), ); } - let internal_endpoint = rule - .endpoints - .iter() - .find(|endpoint| endpoint.host == "internal-admin.local") - .expect("advisor endpoint should be appended"); + assert!( - internal_endpoint.advisor_proposed, - "endpoint provenance must survive merge even when binary provenance is deduped" + !policy_covers_rule(&loaded, &proposed), + "the missing b.example.com × /usr/bin/second pair must fail coverage" + ); + + loaded.network_policies.insert( + "b-second".to_string(), + rule_with_authorizations( + "b-second", + vec![endpoint("b.example.com", 443)], + &["/usr/bin/second"], + ), + ); + assert!( + policy_covers_rule(&loaded, &proposed), + "separate loaded rules may jointly cover the complete Cartesian product" ); } #[test] - fn add_rule_merges_websocket_credential_rewrite_flag() { + fn add_rule_merges_l7_fields_into_existing_endpoint() { let mut policy = restrictive_default_policy(); policy.network_policies.insert( "existing".to_string(), NetworkPolicyRule { name: "existing".to_string(), - endpoints: vec![NetworkEndpoint { - host: "realtime.example.com".to_string(), - port: 443, - ports: vec![443], - protocol: "websocket".to_string(), - access: "read-write".to_string(), + endpoints: vec![endpoint("api.github.com", 443)], + binaries: vec![NetworkBinary { + path: "/usr/bin/curl".to_string(), ..Default::default() }], - ..Default::default() }, ); let incoming = NetworkPolicyRule { name: "incoming".to_string(), endpoints: vec![NetworkEndpoint { - host: "realtime.example.com".to_string(), + host: "api.github.com".to_string(), port: 443, ports: vec![443], - protocol: "websocket".to_string(), - websocket_credential_rewrite: true, + protocol: "rest".to_string(), + enforcement: "enforce".to_string(), + rules: vec![rest_rule("GET", "/repos/**")], ..Default::default() }], - ..Default::default() + binaries: vec![binary("/usr/bin/curl"), binary("/usr/bin/gh")], }; let result = merge_policy( policy, &[PolicyMergeOp::AddRule { - rule_name: "allow_realtime_example_com_443".to_string(), + rule_name: "allow_api_github_com_443".to_string(), rule: incoming, }], ) .expect("merge should succeed"); - let endpoint = &result.policy.network_policies["existing"].endpoints[0]; - assert!(endpoint.websocket_credential_rewrite); + let rule = &result.policy.network_policies["existing"]; + let endpoint = &rule.endpoints[0]; + assert_eq!(endpoint.protocol, "rest"); + assert_eq!(endpoint.enforcement, "enforce"); + assert_eq!(endpoint.rules.len(), 1); + assert_eq!(rule.binaries.len(), 2); } #[test] - fn add_rule_merges_request_body_credential_rewrite_flag() { + fn add_rule_user_binary_clears_advisor_marker_for_same_path() { let mut policy = restrictive_default_policy(); policy.network_policies.insert( "existing".to_string(), NetworkPolicyRule { name: "existing".to_string(), - endpoints: vec![NetworkEndpoint { - host: "slack.com".to_string(), - port: 443, - ports: vec![443], - protocol: "rest".to_string(), - access: "read-write".to_string(), - ..Default::default() - }], - ..Default::default() + endpoints: vec![endpoint("api.github.com", 443)], + binaries: vec![advisor_binary("/usr/bin/curl")], + }, + ); + + let incoming = NetworkPolicyRule { + name: "incoming".to_string(), + endpoints: vec![endpoint("api.github.com", 443)], + binaries: vec![NetworkBinary { + path: "/usr/bin/curl".to_string(), + ..Default::default() + }], + }; + + let result = merge_policy( + policy, + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect("merge should succeed"); + + let rule = &result.policy.network_policies["existing"]; + assert_eq!(rule.binaries.len(), 1); + #[allow(deprecated)] + { + assert!(!rule.binaries[0].harness); + } + } + + #[test] + fn add_rule_duplicate_binaries_prefer_user_declared_marker() { + let incoming = NetworkPolicyRule { + name: "incoming".to_string(), + endpoints: vec![endpoint("api.github.com", 443)], + binaries: vec![ + advisor_binary("/usr/bin/curl"), + NetworkBinary { + path: "/usr/bin/curl".to_string(), + ..Default::default() + }, + ], + }; + + let result = merge_policy( + restrictive_default_policy(), + &[PolicyMergeOp::AddRule { + rule_name: "github".to_string(), + rule: incoming, + }], + ) + .expect("merge should succeed"); + + let rule = &result.policy.network_policies["github"]; + assert_eq!(rule.binaries.len(), 1); + #[allow(deprecated)] + { + assert!(!rule.binaries[0].harness); + } + } + + #[test] + fn add_rule_preserves_advisor_endpoint_marker_when_binary_is_deduped() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "app-api".to_string(), + NetworkPolicyRule { + name: "app-api".to_string(), + endpoints: vec![endpoint("api.example.com", 443)], + binaries: vec![NetworkBinary { + path: "/usr/bin/python".to_string(), + ..Default::default() + }], + }, + ); + + let incoming = NetworkPolicyRule { + name: "app-api".to_string(), + endpoints: vec![NetworkEndpoint { + host: "internal-admin.local".to_string(), + port: 443, + ports: vec![443], + advisor_proposed: true, + ..Default::default() + }], + binaries: vec![advisor_binary("/usr/bin/python")], + }; + + let result = merge_policy( + policy, + &[PolicyMergeOp::AddRule { + rule_name: "app-api".to_string(), + rule: incoming, + }], + ) + .expect("merge should succeed"); + + 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" + ); + } + let internal_endpoint = rule + .endpoints + .iter() + .find(|endpoint| endpoint.host == "internal-admin.local") + .expect("advisor endpoint should be appended"); + assert!( + internal_endpoint.advisor_proposed, + "endpoint provenance must survive merge even when binary provenance is deduped" + ); + } + + #[test] + fn add_rule_merges_websocket_credential_rewrite_flag() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "existing".to_string(), + NetworkPolicyRule { + name: "existing".to_string(), + endpoints: vec![NetworkEndpoint { + host: "realtime.example.com".to_string(), + port: 443, + ports: vec![443], + protocol: "websocket".to_string(), + access: "read-write".to_string(), + ..Default::default() + }], + ..Default::default() + }, + ); + + let incoming = NetworkPolicyRule { + name: "incoming".to_string(), + endpoints: vec![NetworkEndpoint { + host: "realtime.example.com".to_string(), + port: 443, + ports: vec![443], + protocol: "websocket".to_string(), + websocket_credential_rewrite: true, + ..Default::default() + }], + ..Default::default() + }; + + let result = merge_policy( + policy, + &[PolicyMergeOp::AddRule { + rule_name: "allow_realtime_example_com_443".to_string(), + rule: incoming, + }], + ) + .expect("merge should succeed"); + + let endpoint = &result.policy.network_policies["existing"].endpoints[0]; + assert!(endpoint.websocket_credential_rewrite); + } + + #[test] + fn add_rule_merges_request_body_credential_rewrite_flag() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "existing".to_string(), + NetworkPolicyRule { + name: "existing".to_string(), + endpoints: vec![NetworkEndpoint { + host: "slack.com".to_string(), + port: 443, + ports: vec![443], + protocol: "rest".to_string(), + access: "read-write".to_string(), + ..Default::default() + }], + ..Default::default() }, ); @@ -1724,11 +3503,9 @@ mod tests { } #[test] - fn policy_covers_rule_treats_empty_proposed_binaries_as_any_binary() { + fn policy_covers_rule_requires_loaded_any_binary_scope_for_any_binary_proposal() { // A proposed rule with no binaries is the "any binary" shape. - // The merged rule keeps its own binaries; coverage holds iff - // endpoint and (vacuously satisfied) binary set match. Document - // the semantics so a future reader doesn't flip it accidentally. + // A specific loaded binary list cannot cover that Cartesian scope. let proposed = NetworkPolicyRule { name: "any_binary_rule".to_string(), endpoints: vec![endpoint("api.github.com", 443)], @@ -1749,9 +3526,17 @@ mod tests { ); assert!( - policy_covers_rule(&policy, &proposed), - "empty proposed binaries should match any merged binary set" + !policy_covers_rule(&policy, &proposed), + "a specific loaded binary list must not cover an any-binary proposal" ); + + policy + .network_policies + .get_mut("existing") + .expect("existing rule") + .binaries + .clear(); + assert!(policy_covers_rule(&policy, &proposed)); } #[test] @@ -1922,10 +3707,1125 @@ mod tests { got keys: {:?}", result.policy.network_policies.keys().collect::>() ); + // The existing rule declares no binaries, which authorizes any binary. + // Absorbing the incoming path would make the list non-empty and revoke + // every other process, so the wider scope is kept and reported instead. let merged = &result.policy.network_policies["custom_github"]; assert!( - merged.binaries.iter().any(|b| b.path == "/usr/bin/curl"), - "user rule should have absorbed the incoming curl binary" + merged.binaries.is_empty(), + "an any-binary rule must not be narrowed by an additive operation; got {:?}", + merged.binaries + ); + assert!( + result.warnings.iter().any(|warning| matches!( + warning, + PolicyMergeWarning::ExistingAnyBinaryScopeRetained { rule_name, incoming } + if rule_name == "custom_github" + && incoming == &["/usr/bin/curl".to_string()] + )), + "got warnings: {:?}", + result.warnings + ); + } + + fn endpoint_with_ports(host: &str, ports: &[u32]) -> NetworkEndpoint { + NetworkEndpoint { + host: host.to_string(), + port: ports.first().copied().unwrap_or_default(), + ports: ports.to_vec(), + ..Default::default() + } + } + + /// A proposal that omits a field the merge retains still has to read back as + /// covered. The merge keeps the loaded value and reports success, so + /// requiring the proposal to match the loaded value would leave the + /// `policy.local /wait` long poll spinning against a policy that did load. + #[test] + fn coverage_treats_an_omitted_retained_field_as_unspecified() { + for (field, loaded_endpoint) in [ + ( + "enforcement", + NetworkEndpoint { + enforcement: "enforce".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ( + "protocol", + NetworkEndpoint { + protocol: "rest".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ( + "tls", + NetworkEndpoint { + tls: "skip".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ] { + let existing = + rule_with_authorizations("existing", vec![loaded_endpoint], &["/usr/bin/trusted"]); + // The proposal declares the whole binary scope and leaves the + // retained field unset, so the merge accepts it unchanged. + let proposal = rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/trusted"], + ); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: proposal.clone(), + }], + ) + .unwrap_or_else(|error| panic!("omitting {field} should merge cleanly: {error}")); + + assert!( + policy_covers_rule(&result.policy, &proposal), + "a proposal omitting {field} merged cleanly, so it must read back as covered" + ); + } + } + + /// A rule's port list is a set of independent authorizations, so the loaded + /// policy may spread one proposed endpoint's ports across several rules. + #[test] + fn coverage_resolves_each_port_across_the_loaded_rule_union() { + let mut policy = policy_with_rule( + "https", + rule_with_authorizations( + "https", + vec![endpoint_with_ports("api.example.com", &[443])], + &["/usr/bin/client"], + ), + ); + policy.network_policies.insert( + "alt_https".to_string(), + rule_with_authorizations( + "alt_https", + vec![endpoint_with_ports("api.example.com", &[8443])], + &["/usr/bin/client"], + ), + ); + + let proposed = rule_with_authorizations( + "combined", + vec![endpoint_with_ports("api.example.com", &[443, 8443])], + &["/usr/bin/client"], + ); + + assert!( + policy_covers_rule(&policy, &proposed), + "two loaded rules covering one port each authorize the same traffic \ + as one rule covering both" + ); + } + + /// An endpoint that declares no port at all still needs a matching loaded + /// endpoint. Iterating an empty port list would make the coverage `all()` + /// vacuously true and report an unmatched proposal as covered. + #[test] + fn coverage_rejects_a_portless_endpoint_no_loaded_rule_matches() { + let policy = policy_with_rule( + "existing", + rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/client"], + ), + ); + let proposed = rule_with_authorizations( + "other", + vec![NetworkEndpoint { + host: "unrelated.example.com".to_string(), + ..Default::default() + }], + &["/usr/bin/client"], + ); + + assert!( + !policy_covers_rule(&policy, &proposed), + "no loaded rule mentions the proposed host, so coverage must fail closed" + ); + } + + /// A complete declaration may arrive split across several incoming + /// endpoints. Requiring one declaration to cover the whole merged endpoint + /// would reject a new binary that did declare every port it will reach. + #[test] + fn new_binary_may_declare_one_endpoints_ports_across_several_declarations() { + let existing = rule_with_authorizations( + "existing", + vec![endpoint_with_ports("api.example.com", &[443, 8443])], + &["/usr/bin/trusted"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![ + endpoint_with_ports("api.example.com", &[443]), + endpoint_with_ports("api.example.com", &[8443]), + ], + &["/usr/bin/trusted", "/usr/bin/second"], + ); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect("the new binary declared both ports, just in separate endpoints"); + + let merged = &result.policy.network_policies["existing"]; + assert!(merged.binaries.iter().any(|b| b.path == "/usr/bin/second")); + } + + /// The complement: a split declaration that misses a port is still an + /// implicit grant and must be rejected, naming the port left undeclared. + #[test] + fn new_binary_split_declaration_must_still_cover_every_port() { + let existing = rule_with_authorizations( + "existing", + vec![endpoint_with_ports("api.example.com", &[443, 8443])], + &["/usr/bin/trusted"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![endpoint_with_ports("api.example.com", &[443])], + &["/usr/bin/trusted", "/usr/bin/second"], + ); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect_err("port 8443 was never declared for the new binary"); + + assert!(matches!( + error, + PolicyMergeError::NewBinaryWouldInheritAuthorization { ports, .. } + if ports == vec![8443] + )); + } + + /// An incoming empty binary list means any binary, which widens a + /// restricted rule to every binary on the system. It has to declare the + /// rule's whole endpoint scope first, exactly like a new concrete path. + #[test] + fn any_binary_proposal_cannot_inherit_undeclared_endpoints() { + let existing = rule_with_authorizations( + "existing", + vec![ + endpoint("api.example.com", 443), + endpoint("admin.example.com", 443), + ], + &["/usr/bin/trusted"], + ); + let incoming = + rule_with_authorizations("existing", vec![endpoint("api.example.com", 443)], &[]); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect_err("any-binary scope would reach the undeclared admin endpoint"); + + assert!(matches!( + error, + PolicyMergeError::NewBinaryWouldInheritAuthorization { binary_scope, host, .. } + if binary_scope == ANY_BINARY_SCOPE && host == "admin.example.com" + )); + } + + /// Once the declaration is complete the promotion has to be applied. + /// Appending an empty binary list would leave the restricted scope in place, + /// so the operation would report success while authorizing nothing it asked + /// for and coverage would never converge. + #[test] + fn any_binary_proposal_replaces_a_restricted_scope_once_fully_declared() { + let existing = rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/trusted"], + ); + let incoming = + rule_with_authorizations("existing", vec![endpoint("api.example.com", 443)], &[]); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming.clone(), + }], + ) + .expect("the proposal declared the rule's only endpoint"); + + assert!( + result.policy.network_policies["existing"] + .binaries + .is_empty(), + "the any-binary scope the operation asked for must be applied" + ); + assert!( + policy_covers_rule(&result.policy, &incoming), + "an applied any-binary scope must read back as covered" + ); + } + + /// The overlap fallback is a convenience for incremental refinement, not + /// something the operation requested. When folding would grant undeclared + /// authorization, the proposal keeps its own rule name instead, which + /// authorizes exactly the product it declared. Without this the documented + /// "put the new authorization in a separate rule" remediation is + /// unreachable for any endpoint that overlaps an existing rule. + #[test] + fn overlapping_partial_authorization_keeps_its_requested_rule_name() { + let existing = rule_with_authorizations( + "existing", + vec![ + endpoint("api.example.com", 443), + endpoint("admin.example.com", 443), + ], + &["/usr/bin/trusted"], + ); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "narrow_grant".to_string(), + rule: rule_with_authorizations( + "narrow_grant", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/limited"], + ), + }], + ) + .expect("a partial grant must land on its own rule rather than be rejected"); + + let narrow = result + .policy + .network_policies + .get("narrow_grant") + .expect("the requested key must be preserved when folding would widen"); + assert_eq!(narrow.binaries.len(), 1); + assert_eq!(narrow.endpoints.len(), 1); + + // Skipping the fold has to be visible: the operator asked for one rule + // and got two, and later host-and-port operations now have two + // candidate rules to resolve against. + assert!( + result.warnings.iter().any(|warning| matches!( + warning, + PolicyMergeWarning::KeptRequestedRuleNameToAvoidWidening { + rule_name, + overlapping_rule_name, + .. + } if rule_name == "narrow_grant" && overlapping_rule_name == "existing" + )), + "got warnings: {:?}", + result.warnings + ); + + // The existing rule keeps its own product untouched: the limited binary + // never gains the admin endpoint. + let untouched = &result.policy.network_policies["existing"]; + assert_eq!(untouched.binaries.len(), 1); + assert!( + untouched + .binaries + .iter() + .all(|b| b.path != "/usr/bin/limited") + ); + } + + /// Folding stays the default when it grants nothing new, so incremental + /// refinement under a fresh rule name still consolidates. + #[test] + fn overlapping_complete_authorization_still_folds_into_the_existing_rule() { + let existing = rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/trusted"], + ); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "refinement".to_string(), + rule: rule_with_authorizations( + "refinement", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/trusted", "/usr/bin/second"], + ), + }], + ) + .expect("the operation declared the rule's whole scope"); + + assert!( + !result.policy.network_policies.contains_key("refinement"), + "a complete declaration should still fold into the overlapping rule" + ); + assert_eq!(result.policy.network_policies["existing"].binaries.len(), 2); + } + + /// An MCP contract conflict is not resolved by moving the authorization to + /// another rule, because the supervisor establishes one contract per host + /// and port. It must propagate even on the fallback path. + #[test] + fn overlapping_mcp_contract_conflict_is_not_deflected_to_a_separate_rule() { + let existing = rule_with_authorizations( + "existing", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 65536, + vec![mcp_tool_rule("existing-tool")], + )], + &["/usr/bin/trusted"], + ); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "second_contract".to_string(), + rule: rule_with_authorizations( + "second_contract", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 131_072, + vec![mcp_tool_rule("other-tool")], + )], + &["/usr/bin/trusted"], + ), + }], + ) + .expect_err("one host and port cannot carry two inspection contracts"); + + assert!(matches!( + error, + PolicyMergeError::McpContractConflict { .. } + )); + } + + /// The round-trip property the whole design rests on: whatever the gateway + /// accepts must immediately read back as covered, or `/wait` hangs. + #[test] + fn every_accepted_merge_reads_back_as_covered() { + let cases: Vec<(&str, NetworkPolicyRule, NetworkPolicyRule)> = vec![ + ( + "omitted enforcement against an enforced endpoint", + rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + enforcement: "enforce".to_string(), + ..endpoint("api.example.com", 443) + }], + &["/usr/bin/trusted"], + ), + rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/trusted"], + ), + ), + ( + "split ports declared across two endpoints", + rule_with_authorizations( + "existing", + vec![endpoint_with_ports("api.example.com", &[443, 8443])], + &["/usr/bin/trusted"], + ), + rule_with_authorizations( + "existing", + vec![ + endpoint_with_ports("api.example.com", &[443]), + endpoint_with_ports("api.example.com", &[8443]), + ], + &["/usr/bin/trusted", "/usr/bin/second"], + ), + ), + ( + "any-binary promotion", + rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/trusted"], + ), + rule_with_authorizations("existing", vec![endpoint("api.example.com", 443)], &[]), + ), + ]; + + for (label, existing, proposal) in cases { + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: proposal.clone(), + }], + ) + .unwrap_or_else(|error| panic!("{label} should merge: {error}")); + + assert!( + policy_covers_rule(&result.policy, &proposal), + "{label}: the merge was accepted, so coverage must report it loaded" + ); + } + } + + /// A declaration that leaves a retained field unset expresses no opinion + /// about it. Rejecting on that field would refuse a complete declaration + /// over a mode the operation never tried to change, and for enforcement it + /// would refuse specifically because the binary receives the stricter mode. + #[test] + fn declaration_may_omit_retained_fields_it_does_not_change() { + for (field, loaded_endpoint) in [ + ( + "enforcement", + NetworkEndpoint { + enforcement: "enforce".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ( + "protocol", + NetworkEndpoint { + protocol: "rest".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ( + "tls", + NetworkEndpoint { + tls: "skip".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ( + "access", + NetworkEndpoint { + protocol: "rest".to_string(), + access: "read-only".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ( + "credential_signing", + NetworkEndpoint { + credential_signing: "sigv4".to_string(), + signing_service: "s3".to_string(), + signing_region: "us-west-2".to_string(), + ..endpoint("api.example.com", 443) + }, + ), + ( + "persisted_queries", + NetworkEndpoint { + persisted_queries: "allow_registered".to_string(), + graphql_max_body_bytes: 4096, + ..endpoint("api.example.com", 443) + }, + ), + ( + "json_rpc_max_body_bytes", + NetworkEndpoint { + json_rpc_max_body_bytes: 65536, + ..endpoint("api.example.com", 443) + }, + ), + ] { + let existing = + rule_with_authorizations("existing", vec![loaded_endpoint], &["/usr/bin/trusted"]); + // Declares the whole scope, but says nothing about the carry-over + // field the endpoint already carries. + let incoming = rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/trusted", "/usr/bin/second"], + ); + + merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .unwrap_or_else(|error| { + panic!("a declaration omitting {field} covers the whole scope: {error}") + }); + } + } + + /// Omitting a retained field is not the same as contradicting one. A + /// declaration that names a different protocol still has to be rejected. + #[test] + fn declaration_that_contradicts_a_retained_field_is_still_rejected() { + let existing = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + ..endpoint("api.example.com", 443) + }], + &["/usr/bin/trusted"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + protocol: "websocket".to_string(), + ..endpoint("api.example.com", 443) + }], + &["/usr/bin/trusted", "/usr/bin/second"], + ); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect_err("the declaration describes a protocol the merge will not apply"); + + assert!(matches!( + error, + PolicyMergeError::NewBinaryWouldInheritAuthorization { .. } + )); + } + + /// `AddAllowRules` and `AddDenyRules` name their target by host and port + /// alone, so when two rules carry that endpoint they cannot say which binary + /// scope to widen. Picking one silently would add the rule to whichever key + /// sorts first. + #[test] + fn l7_operations_reject_an_endpoint_carried_by_several_rules() { + let mut policy = policy_with_rule( + "broad", + rule_with_authorizations( + "broad", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/public")], + ..endpoint("api.example.com", 443) + }], + &["/usr/bin/trusted", "/usr/bin/limited"], + ), + ); + policy.network_policies.insert( + "narrow".to_string(), + rule_with_authorizations( + "narrow", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/public")], + ..endpoint("api.example.com", 443) + }], + &["/usr/bin/other"], + ), + ); + + for operation in [ + PolicyMergeOp::AddAllowRules { + host: "api.example.com".to_string(), + port: 443, + rules: vec![rest_rule("POST", "/admin")], + }, + PolicyMergeOp::AddDenyRules { + host: "api.example.com".to_string(), + port: 443, + deny_rules: Vec::new(), + }, + ] { + let error = merge_policy(policy.clone(), &[operation]) + .expect_err("two rules carry this endpoint, so the target is ambiguous"); + + assert!( + matches!( + &error, + PolicyMergeError::AmbiguousEndpointRule { targets, .. } + if targets == &["broad".to_string(), "narrow".to_string()] + ), + "got {error:?}" + ); + } + } + + /// A single rule can own several endpoints on one host and port, because + /// `endpoints_overlap` treats a different path as a different endpoint. + /// Counting owning rules would miss this and let the operation land on + /// whichever endpoint happens to sit first in the vector. + #[test] + fn l7_operations_reject_two_paths_on_one_host_and_port_within_a_rule() { + let policy = policy_with_rule( + "versioned", + rule_with_authorizations( + "versioned", + vec![ + NetworkEndpoint { + protocol: "rest".to_string(), + path: "/v1".to_string(), + rules: vec![rest_rule("GET", "/public")], + ..endpoint("api.example.com", 443) + }, + NetworkEndpoint { + protocol: "rest".to_string(), + path: "/v2".to_string(), + rules: vec![rest_rule("GET", "/public")], + ..endpoint("api.example.com", 443) + }, + ], + &["/usr/bin/trusted"], + ), + ); + + let error = merge_policy( + policy, + &[PolicyMergeOp::AddAllowRules { + host: "api.example.com".to_string(), + port: 443, + rules: vec![rest_rule("POST", "/admin")], + }], + ) + .expect_err("one host and port resolves to two endpoints on this rule"); + + assert!( + matches!( + &error, + PolicyMergeError::AmbiguousEndpointRule { targets, .. } + if targets == &[ + "versioned (path '/v1')".to_string(), + "versioned (path '/v2')".to_string(), + ] + ), + "got {error:?}" + ); + } + + /// The unambiguous case must keep working, or every incremental L7 update + /// breaks. + #[test] + fn l7_operations_still_apply_when_one_rule_carries_the_endpoint() { + let policy = policy_with_rule( + "only", + rule_with_authorizations( + "only", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/public")], + ..endpoint("api.example.com", 443) + }], + &["/usr/bin/trusted"], + ), + ); + + let result = merge_policy( + policy, + &[PolicyMergeOp::AddAllowRules { + host: "api.example.com".to_string(), + port: 443, + rules: vec![rest_rule("POST", "/issues")], + }], + ) + .expect("a single owning rule is unambiguous"); + + assert_eq!( + result.policy.network_policies["only"].endpoints[0] + .rules + .len(), + 2 + ); + } + + /// Removing a binary from an any-binary rule has no entry to drop, so + /// reporting success would leave the operator believing a revocation landed + /// while the rule still authorizes every binary. + #[test] + fn remove_binary_rejects_an_any_binary_rule_instead_of_doing_nothing() { + let policy = policy_with_rule( + "wide", + rule_with_authorizations("wide", vec![endpoint("api.example.com", 443)], &[]), + ); + + let error = merge_policy( + policy, + &[PolicyMergeOp::RemoveBinary { + rule_name: "wide".to_string(), + binary_path: "/usr/bin/untrusted".to_string(), + }], + ) + .expect_err("an any-binary rule has no binary entry to remove"); + + assert!(matches!( + error, + PolicyMergeError::CannotRemoveBinaryFromAnyBinaryScope { rule_name, binary_path, .. } + if rule_name == "wide" && binary_path == "/usr/bin/untrusted" + )); + } + + /// Removing the last named binary deletes the rule rather than leaving an + /// empty list, which would silently widen it to every binary. + #[test] + fn remove_binary_deletes_the_rule_rather_than_widening_it() { + let policy = policy_with_rule( + "narrow", + rule_with_authorizations( + "narrow", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/only"], + ), + ); + + let result = merge_policy( + policy, + &[PolicyMergeOp::RemoveBinary { + rule_name: "narrow".to_string(), + binary_path: "/usr/bin/only".to_string(), + }], + ) + .expect("removing the last binary is a valid revocation"); + + assert!( + !result.policy.network_policies.contains_key("narrow"), + "an emptied rule must be removed, not left authorizing any binary" + ); + } + + /// Widened fields merge into an endpoint as a whole, so a change declared + /// for one port of a multi-port endpoint reaches the endpoint's other ports. + /// Declaring every existing binary must not exempt the operation from + /// naming the ports its change lands on. + #[test] + fn declaring_every_binary_does_not_license_an_undeclared_port() { + let existing = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/x")], + ..endpoint_with_ports("api.example.com", &[443, 8443]) + }], + &["/usr/bin/only"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("POST", "/y")], + ..endpoint_with_ports("api.example.com", &[443]) + }], + &["/usr/bin/only"], + ); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect_err("POST would also land on the undeclared port 8443"); + + assert!( + matches!( + &error, + PolicyMergeError::UndeclaredPortWouldChange { ports, host, .. } + if ports == &[8443] && host == "api.example.com" + ), + "got {error:?}" + ); + } + + /// Naming every port the change lands on is accepted, so the incremental + /// flow still works once the operation is explicit about its scope. + #[test] + fn naming_every_changed_port_is_accepted() { + let existing = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("GET", "/x")], + ..endpoint_with_ports("api.example.com", &[443, 8443]) + }], + &["/usr/bin/only"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + protocol: "rest".to_string(), + rules: vec![rest_rule("POST", "/y")], + ..endpoint_with_ports("api.example.com", &[443, 8443]) + }], + &["/usr/bin/only"], + ); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect("the operation named both ports the change reaches"); + + assert_eq!( + result.policy.network_policies["existing"].endpoints[0] + .rules + .len(), + 2 + ); + } + + /// The supervisor resolves one MCP contract per host and port, matching on + /// host and port without consulting the path, so two contracts cannot + /// coexist even under different paths on the same rule. + #[test] + fn two_mcp_contracts_on_one_host_and_port_are_rejected_across_paths() { + let existing = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + path: "/a".to_string(), + ..mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 65536, + vec![mcp_tool_rule("first")], + ) + }], + &["/usr/bin/only"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + path: "/b".to_string(), + ..mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 131_072, + vec![mcp_tool_rule("second")], + ) + }], + &["/usr/bin/only"], + ); + + let error = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect_err("a differing path does not license a second inspection contract"); + + assert!( + matches!( + &error, + PolicyMergeError::ConflictingMcpContractsForEndpoint { host, port, contracts } + if host == "mcp.example.com" && *port == 443 && contracts.len() == 2 + ), + "got {error:?}" + ); + } + + /// The same conflict across two rules is equally unsafe, because the + /// supervisor does not care which rule contributed the endpoint. + #[test] + fn two_mcp_contracts_on_one_host_and_port_are_rejected_across_rules() { + let policy = policy_with_rule( + "first_rule", + rule_with_authorizations( + "first_rule", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 65536, + vec![mcp_tool_rule("first")], + )], + &["/usr/bin/a"], + ), + ); + + let error = merge_policy( + policy, + &[PolicyMergeOp::AddRule { + rule_name: "second_rule".to_string(), + rule: rule_with_authorizations( + "second_rule", + vec![NetworkEndpoint { + path: "/other".to_string(), + ..mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 131_072, + vec![mcp_tool_rule("second")], + ) + }], + &["/usr/bin/b"], + ), + }], + ) + .expect_err("a separate rule does not license a second inspection contract"); + + assert!(matches!( + error, + PolicyMergeError::ConflictingMcpContractsForEndpoint { .. } + )); + } + + /// Matching contracts on one host and port are fine, so splitting an MCP + /// surface across paths or rules stays possible when the contract agrees. + #[test] + fn matching_mcp_contracts_on_one_host_and_port_are_accepted() { + let existing = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + path: "/a".to_string(), + ..mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 65536, + vec![mcp_tool_rule("first")], + ) + }], + &["/usr/bin/only"], + ); + let incoming = rule_with_authorizations( + "existing", + vec![NetworkEndpoint { + path: "/b".to_string(), + ..mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 65536, + vec![mcp_tool_rule("second")], + ) + }], + &["/usr/bin/only"], + ); + + merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming, + }], + ) + .expect("identical contracts resolve the same way whichever endpoint matches first"); + } + + /// A conflict already present in the baseline must not make every later + /// update fail with an error the operation did nothing to cause. + #[test] + fn a_preexisting_mcp_conflict_does_not_block_an_unrelated_update() { + let mut policy = policy_with_rule( + "first_rule", + rule_with_authorizations( + "first_rule", + vec![mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 65536, + vec![mcp_tool_rule("first")], + )], + &["/usr/bin/a"], + ), + ); + policy.network_policies.insert( + "second_rule".to_string(), + rule_with_authorizations( + "second_rule", + vec![NetworkEndpoint { + path: "/other".to_string(), + ..mcp_endpoint( + "mcp.example.com", + &[443], + None, + None, + 131_072, + vec![mcp_tool_rule("second")], + ) + }], + &["/usr/bin/b"], + ), + ); + + merge_policy( + policy, + &[PolicyMergeOp::AddRule { + rule_name: "unrelated".to_string(), + rule: rule_with_authorizations( + "unrelated", + vec![endpoint("other.example.com", 443)], + &["/usr/bin/c"], + ), + }], + ) + .expect("an unrelated endpoint must not inherit a baseline conflict"); + } + + /// An additive operation must never revoke access. Appending a named binary + /// to a rule that authorizes any binary would restrict it to that one path. + #[test] + fn naming_binaries_does_not_narrow_an_any_binary_rule() { + let existing = + rule_with_authorizations("existing", vec![endpoint("api.example.com", 443)], &[]); + let incoming = rule_with_authorizations( + "existing", + vec![endpoint("api.example.com", 443)], + &["/usr/bin/a"], + ); + + let result = merge_policy( + policy_with_rule("existing", existing), + &[PolicyMergeOp::AddRule { + rule_name: "existing".to_string(), + rule: incoming.clone(), + }], + ) + .expect("naming a binary already covered by any-binary is additive"); + + assert!( + result.policy.network_policies["existing"] + .binaries + .is_empty(), + "the any-binary scope must survive; got {:?}", + result.policy.network_policies["existing"].binaries + ); + assert!(result.warnings.iter().any(|warning| matches!( + warning, + PolicyMergeWarning::ExistingAnyBinaryScopeRetained { .. } + ))); + assert!( + policy_covers_rule(&result.policy, &incoming), + "an any-binary rule covers the named binary, so coverage must converge" ); } } diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 53124261e6..72c040fdb4 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -4016,11 +4016,21 @@ fn validate_merge_operations_for_server(operations: &[PolicyMergeOp]) -> Result< fn map_policy_merge_error(error: openshell_policy::PolicyMergeError) -> Status { match error { openshell_policy::PolicyMergeError::MissingRuleNameForAddRule + | openshell_policy::PolicyMergeError::EmptyAddRuleEndpoints { .. } | openshell_policy::PolicyMergeError::InvalidEndpointReference { .. } | openshell_policy::PolicyMergeError::UnsupportedAccessPreset { .. } => { Status::invalid_argument(error.to_string()) } - openshell_policy::PolicyMergeError::EndpointNotFound { .. } + openshell_policy::PolicyMergeError::McpContractConflict { .. } + | openshell_policy::PolicyMergeError::NewBinaryWouldInheritAuthorization { .. } + | openshell_policy::PolicyMergeError::ExistingBinariesWouldInheritAuthorization { + .. + } + | openshell_policy::PolicyMergeError::UndeclaredPortWouldChange { .. } + | openshell_policy::PolicyMergeError::ConflictingMcpContractsForEndpoint { .. } + | openshell_policy::PolicyMergeError::AmbiguousEndpointRule { .. } + | openshell_policy::PolicyMergeError::CannotRemoveBinaryFromAnyBinaryScope { .. } + | openshell_policy::PolicyMergeError::EndpointNotFound { .. } | openshell_policy::PolicyMergeError::EndpointHasNoL7Inspection { .. } | openshell_policy::PolicyMergeError::UnsupportedEndpointProtocol { .. } | openshell_policy::PolicyMergeError::EndpointHasNoAllowBase { .. } => { @@ -4785,6 +4795,103 @@ mod tests { assert!(err.message().contains("reserved '_provider_' prefix")); } + #[test] + fn policy_merge_error_mapping_distinguishes_request_shape_from_state_conflicts() { + let empty = + map_policy_merge_error(openshell_policy::PolicyMergeError::EmptyAddRuleEndpoints { + operation_index: 0, + rule_name: "empty".to_string(), + }); + assert_eq!(empty.code(), Code::InvalidArgument); + + let contract = + map_policy_merge_error(openshell_policy::PolicyMergeError::McpContractConflict { + operation_index: 1, + host: "mcp.example.com".to_string(), + port: 443, + existing: "mcp(max_body_bytes=65536)".to_string(), + incoming: "mcp(max_body_bytes=131072)".to_string(), + }); + assert_eq!(contract.code(), Code::FailedPrecondition); + + let inheritance = map_policy_merge_error( + openshell_policy::PolicyMergeError::NewBinaryWouldInheritAuthorization { + operation_index: 2, + rule_name: "existing".to_string(), + binary_scope: "binary '/usr/bin/client'".to_string(), + host: "mcp.example.com".to_string(), + ports: vec![443], + }, + ); + assert_eq!(inheritance.code(), Code::FailedPrecondition); + // The proposer has to know which binary scope triggered the rejection. + assert!(inheritance.message().contains("/usr/bin/client")); + + let existing_scope = map_policy_merge_error( + openshell_policy::PolicyMergeError::ExistingBinariesWouldInheritAuthorization { + operation_index: 3, + rule_name: "existing".to_string(), + host: "api.example.com".to_string(), + ports: vec![443], + undeclared_binaries: vec!["/usr/bin/other".to_string()], + }, + ); + assert_eq!(existing_scope.code(), Code::FailedPrecondition); + // The proposer has to know which binaries to add, so the remediation + // detail must survive into the status message. + assert!(existing_scope.message().contains("/usr/bin/other")); + + // Both of these describe a well-formed request the current policy state + // forbids, so they are preconditions rather than argument errors. + let ambiguous = + map_policy_merge_error(openshell_policy::PolicyMergeError::AmbiguousEndpointRule { + host: "api.example.com".to_string(), + port: 443, + targets: vec!["broad".to_string(), "narrow".to_string()], + }); + assert_eq!(ambiguous.code(), Code::FailedPrecondition); + // The operator has to know which rules collide to pick a way forward. + assert!(ambiguous.message().contains("broad")); + assert!(ambiguous.message().contains("narrow")); + + let undeclared_port = map_policy_merge_error( + openshell_policy::PolicyMergeError::UndeclaredPortWouldChange { + operation_index: 5, + rule_name: "existing".to_string(), + host: "api.example.com".to_string(), + ports: vec![8443], + }, + ); + assert_eq!(undeclared_port.code(), Code::FailedPrecondition); + // The proposer has to know which port to declare. + assert!(undeclared_port.message().contains("8443")); + + let mcp_conflict = map_policy_merge_error( + openshell_policy::PolicyMergeError::ConflictingMcpContractsForEndpoint { + host: "mcp.example.com".to_string(), + port: 443, + contracts: vec![ + "mcp(strict_tool_names=true, allow_all_known_mcp_methods=false, max_body_bytes=65536)" + .to_string(), + "mcp(strict_tool_names=true, allow_all_known_mcp_methods=false, max_body_bytes=131072)" + .to_string(), + ], + }, + ); + assert_eq!(mcp_conflict.code(), Code::FailedPrecondition); + assert!(mcp_conflict.message().contains("131072")); + + let any_binary = map_policy_merge_error( + openshell_policy::PolicyMergeError::CannotRemoveBinaryFromAnyBinaryScope { + operation_index: 4, + rule_name: "wide".to_string(), + binary_path: "/usr/bin/untrusted".to_string(), + }, + ); + assert_eq!(any_binary.code(), Code::FailedPrecondition); + assert!(any_binary.message().contains("/usr/bin/untrusted")); + } + // ---- Sandbox IDOR guard (issue #1354) ---- #[tokio::test] diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 990f260cea..17a65c18c9 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -462,7 +462,25 @@ The CLI validates the argument shapes before it sends the request. The gateway t - a port is outside `1` through `65535`. - `--add-allow` or `--add-deny` points at an endpoint that does not exist. - `--add-allow` or `--add-deny` targets an endpoint that is neither REST nor WebSocket. +- `--add-allow` or `--add-deny` targets a host and port that resolves to more than one endpoint. - `--add-deny` targets an endpoint that has no base allow set. +- an update names an existing rule and adds a binary to it without declaring every endpoint and port that rule already authorizes. +- an update names an existing rule and adds or changes an endpoint on it without declaring every binary that rule already authorizes. +- an update widens a rule to any binary without declaring every endpoint that rule already authorizes. +- an update changes an endpoint without declaring every port that endpoint carries. +- an update gives one host and port two different MCP inspection contracts, including through separate rules or different paths. + +A rule authorizes every listed binary to reach every listed endpoint and port, so merging a binary and an endpoint into the same rule authorizes that pair too. The gateway rejects the whole batch rather than granting a pair the update did not ask for. The error names the binary scope and the ports involved, and lists the binaries you still need to declare. An empty binary list means any binary, so widening a rule to any binary is subject to the same requirement. + +You can declare the scope across several `--add-endpoint` arguments. The update is complete as long as every binary-to-port pair the merged rule ends up authorizing appears somewhere in the update. + +An endpoint's allow rules, deny rules, and allowed IPs apply to every port that endpoint carries, so an update that changes any of them has to name every one of those ports. Declaring `api.example.com:443` alone on an endpoint that also serves `8443` is rejected, because the change would reach `8443` as well. Declaring every existing binary does not lift this requirement; the two are separate axes of the same product. + +The sandbox resolves one MCP inspection contract per host and port, and it matches on host and port without consulting the path. Two MCP endpoints on the same host and port therefore cannot carry different strict-tool-name, method-profile, or body-limit settings, even under different paths or in separate rules. An update that would create that state is rejected. A policy that already contains such a conflict is left alone so unrelated updates still apply, but it should be repaired with full YAML replacement. + +To grant one binary access to only part of an existing rule's endpoints, send it under its own `--rule-name`. The gateway normally folds an update into an existing rule that shares an endpoint, but it keeps your rule name whenever folding would grant authorization you did not declare, so the narrow grant lands as its own rule authorizing exactly what you asked for. The update reports that it kept your rule name and names the rule it would otherwise have folded into. An MCP contract conflict is the exception: one host and port carry a single inspection contract regardless of which rule holds them, so a conflicting update is rejected rather than moved to a separate rule. + +Once a host and port appears in more than one rule, `--add-allow` and `--add-deny` can no longer target it. They select an endpoint by host and port alone, so they cannot say which rule's binary scope to widen, and the gateway rejects the update rather than guessing. The same applies when one rule carries two endpoints on that host and port under different paths. Use full YAML replacement to change L7 rules on an endpoint that appears more than once. ## Global Policy Override