From ee79082165d09f4ce88edf6123cbaf99603920b1 Mon Sep 17 00:00:00 2001 From: Shiju Date: Tue, 3 Nov 2026 17:11:06 +0530 Subject: [PATCH] fix(policy)!: keep MCP tool selectors in agent proposals Agent policy proposals currently lose MCP tool selectors before approval. A proposal intended for one `tools/call` tool can therefore become broader than the agent requested. This change preserves supported selectors through parsing, review and runtime validation, and rejects fields that cannot be applied. Signed-off-by: Shiju --- crates/openshell-cli/src/run.rs | 124 +++++++++++- crates/openshell-policy/src/lib.rs | 13 +- .../openshell-supervisor-network/src/opa.rs | 129 ++++++++---- .../src/policy_local.rs | 185 +++++++++++++----- .../src/skills/policy_advisor.md | 2 + docs/how-it-works/policies/advisor.mdx | 19 +- 6 files changed, 365 insertions(+), 107 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index eb224685ed..034f705909 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -7082,9 +7082,9 @@ fn format_endpoints(rule: &openshell_core::proto::NetworkPolicyRule) -> String { /// `openshell rule get` useful for approval review. fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String { let host_port = if endpoint.port > 0 { - format!("{}:{}", endpoint.host, endpoint.port) + format!("{}:{}{}", endpoint.host, endpoint.port, endpoint.path) } else { - endpoint.host.clone() + format!("{}{}", endpoint.host, endpoint.path) }; let mut tags: Vec = Vec::new(); @@ -7108,16 +7108,67 @@ fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String if let Some(allow) = &r.allow { let method = non_empty_or(&allow.method, "*"); let path = non_empty_or(&allow.path, "*"); - tags.push(format!("allow {method} {path}")); + let mut tag = format!("allow {method} {path}"); + append_graphql( + &mut tag, + &allow.operation_type, + &allow.operation_name, + &allow.fields, + ); + append_matchers(&mut tag, &allow.query); + append_matchers(&mut tag, &allow.params); + tags.push(tag); } } for r in &endpoint.deny_rules { let method = non_empty_or(&r.method, "*"); let path = non_empty_or(&r.path, "*"); - tags.push(format!("deny {method} {path}")); + let mut tag = format!("deny {method} {path}"); + append_graphql(&mut tag, &r.operation_type, &r.operation_name, &r.fields); + append_matchers(&mut tag, &r.query); + append_matchers(&mut tag, &r.params); + tags.push(tag); } format!("{host_port} [{}]", tags.join(", ")) + .escape_debug() + .to_string() +} + +/// Append GraphQL operation selectors, such as `graphql mutation createIssue`. +fn append_graphql(tag: &mut String, operation_type: &str, operation_name: &str, fields: &[String]) { + use std::fmt::Write as _; + + if !operation_type.is_empty() || !operation_name.is_empty() { + let _ = write!( + tag, + " graphql {} {}", + non_empty_or(operation_type, "*"), + non_empty_or(operation_name, "*") + ); + } + if !fields.is_empty() { + let _ = write!(tag, " fields {}", fields.join(",")); + } +} + +/// Append query or MCP params matchers in key order, so an MCP tool rule +/// shows as `allow tools/call * name=read_wiki_structure`. +fn append_matchers( + tag: &mut String, + matchers: &HashMap, +) { + use std::fmt::Write as _; + + let mut entries: Vec<_> = matchers.iter().collect(); + entries.sort_by_key(|(key, _)| *key); + for (key, matcher) in entries { + if matcher.any.is_empty() { + let _ = write!(tag, " {key}={}", matcher.glob); + } else { + let _ = write!(tag, " {key} in [{}]", matcher.any.join(",")); + } + } } #[cfg(test)] @@ -8603,6 +8654,71 @@ mod tests { format_endpoint(&l7_scoped), "host.example.test:443 [L7 rest, allow PUT /v1/example/resource, deny DELETE /v1/example/resource]" ); + + let mcp_tool = NetworkEndpoint { + host: "mcp.example.test".to_string(), + port: 443, + path: "/mcp".to_string(), + protocol: "mcp".to_string(), + rules: vec![L7Rule { + allow: Some(L7Allow { + method: "tools/call".to_string(), + params: std::collections::HashMap::from([( + "name".to_string(), + openshell_core::proto::L7QueryMatcher { + glob: "read_wiki_structure".to_string(), + any: vec![], + }, + )]), + ..Default::default() + }), + }], + ..Default::default() + }; + let graphql = NetworkEndpoint { + host: "api.example.test".to_string(), + port: 443, + protocol: "graphql".to_string(), + rules: vec![L7Rule { + allow: Some(L7Allow { + operation_type: "query".to_string(), + operation_name: "Viewer".to_string(), + ..Default::default() + }), + }], + ..Default::default() + }; + assert_eq!( + format_endpoint(&graphql), + "api.example.test:443 [graphql, allow * * graphql query Viewer]" + ); + + assert_eq!( + format_endpoint(&mcp_tool), + "mcp.example.test:443/mcp [mcp, allow tools/call * name=read_wiki_structure]" + ); + + let mut untrusted = mcp_tool; + let allow = untrusted.rules[0].allow.as_mut().unwrap(); + allow.params.get_mut("name").unwrap().any = vec![ + "*".to_string(), + "\r\u{1b}[2Kname=read_wiki_structure".to_string(), + ]; + allow.params.insert( + "line\n".to_string(), + openshell_core::proto::L7QueryMatcher { + glob: "\r\u{1b}[2K".to_string(), + any: vec![], + }, + ); + allow.operation_name = "\r\u{1b}[2K".to_string(); + allow.fields = vec!["field\n".to_string()]; + let rendered = format_endpoint(&untrusted); + assert!(!rendered.chars().any(char::is_control)); + assert!(rendered.contains(r"name in [*,\r\u{1b}[2Kname=read_wiki_structure]")); + assert!(rendered.contains(r"line\n=\r\u{1b}[2K")); + assert!(rendered.contains(r"graphql * \r\u{1b}[2K")); + assert!(rendered.contains(r"fields field\n")); } #[test] diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index bad8532cdf..80dc30f0dc 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -279,7 +279,8 @@ fn params_with_tool( params } -fn allow_def_to_proto(_protocol: &str, allow: L7AllowDef) -> L7Allow { +/// Convert an authored L7 allow rule to protobuf, carrying MCP `tool` as `params.name`. +pub fn allow_def_to_proto(allow: L7AllowDef) -> L7Allow { let params = flatten_param_matchers(params_with_tool(allow.params, allow.tool)); L7Allow { method: allow.method, @@ -300,7 +301,8 @@ fn allow_def_to_proto(_protocol: &str, allow: L7AllowDef) -> L7Allow { } } -fn deny_def_to_proto(_protocol: &str, deny: L7DenyRuleDef) -> L7DenyRule { +/// Convert an authored L7 deny rule to protobuf, carrying MCP `tool` as `params.name`. +pub fn deny_def_to_proto(deny: L7DenyRuleDef) -> L7DenyRule { let params = flatten_param_matchers(params_with_tool(deny.params, deny.tool)); L7DenyRule { method: deny.method, @@ -534,14 +536,11 @@ fn to_proto(raw: PolicyFile) -> Result { rules: allow_rules .into_iter() .map(|r| L7Rule { - allow: Some(allow_def_to_proto(&protocol, r.allow)), + allow: Some(allow_def_to_proto(r.allow)), }) .collect(), allowed_ips: e.allowed_ips, - deny_rules: deny_rules - .into_iter() - .map(|deny| deny_def_to_proto(&protocol, deny)) - .collect(), + deny_rules: deny_rules.into_iter().map(deny_def_to_proto).collect(), allow_encoded_slash: e.allow_encoded_slash, websocket_credential_rewrite: e.websocket_credential_rewrite, request_body_credential_rewrite: e.request_body_credential_rewrite, diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 2d1a73a949..db9500db79 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -469,47 +469,38 @@ impl OpaEngine { entrypoint_pid: u32, require_binary_identity: bool, ) -> Result { - // Protobuf cannot distinguish an omitted repeated MCP version field - // from an empty one. Canonicalize before any runtime consumer reads - // the policy so both representations select the pinned default. - let proto = openshell_policy::validate_and_canonicalize_sandbox_policy(proto.clone()) - .map_err(|error| { - miette::miette!(render_bounded_validation_diagnostics( - "policy validation failed", - error - .violations() - .iter() - .map(redacted_policy_violation_category), - )) - })?; - - let ambiguities = openshell_policy::find_endpoint_ambiguities(&proto); - if !ambiguities.is_empty() { - return Err(miette::miette!(render_repeated_validation_diagnostic( - "network endpoint ambiguity validation failed", - ambiguities.len(), - "ambiguous network endpoint selectors", - ))); - } - + let (mut data, warnings) = + validated_proto_data(proto, entrypoint_pid, require_binary_identity).map_err( + |invalid| match invalid { + ProtoPolicyInvalid::Policy(error) => { + miette::miette!(render_bounded_validation_diagnostics( + "policy validation failed", + error + .violations() + .iter() + .map(redacted_policy_violation_category), + )) + } + ProtoPolicyInvalid::Ambiguous(ambiguities) => { + miette::miette!(render_repeated_validation_diagnostic( + "network endpoint ambiguity validation failed", + ambiguities.len(), + "ambiguous network endpoint selectors", + )) + } + ProtoPolicyInvalid::L7(errors) => { + miette::miette!(render_repeated_validation_diagnostic( + "L7 policy validation failed", + errors.len(), + "invalid L7 policy configuration", + )) + } + ProtoPolicyInvalid::Internal => { + miette::miette!("internal: failed to parse proto JSON") + } + }, + )?; emit_binary_identity_mode(require_binary_identity, "proto"); - let data_json_str = proto_to_opa_data_json(&proto, entrypoint_pid); - - // Parse back to Value for preprocessing, then re-serialize - let mut data: serde_json::Value = serde_json::from_str(&data_json_str) - .map_err(|_| miette::miette!("internal: failed to parse proto JSON"))?; - inject_runtime_policy_data(&mut data, require_binary_identity); - normalize_endpoint_protocols(&mut data); - - // Validate BEFORE expanding presets - let (errors, warnings) = crate::l7::validate_l7_policies(&data); - if !errors.is_empty() { - return Err(miette::miette!(render_repeated_validation_diagnostic( - "L7 policy validation failed", - errors.len(), - "invalid L7 policy configuration", - ))); - } // Rejected candidates must not leak authored values through warnings. emit_l7_config_warnings(&warnings, "L7 policy validation warning"); @@ -2237,6 +2228,64 @@ fn l7_matchers_to_json( .collect() } +/// Why [`validated_proto_data`] rejected a policy. The messages name authored +/// values, so the runtime loader redacts them; policy.local returns them to +/// the agent that wrote the proposal. +#[derive(Debug)] +pub(crate) enum ProtoPolicyInvalid { + Policy(openshell_policy::PolicyValidationError), + Ambiguous(Vec), + L7(Vec), + Internal, +} + +impl std::fmt::Display for ProtoPolicyInvalid { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Policy(error) => write!(f, "{error}"), + Self::Ambiguous(ambiguities) => { + let messages: Vec = ambiguities.iter().map(ToString::to_string).collect(); + write!(f, "{}", messages.join("; ")) + } + Self::L7(errors) => write!(f, "{}", errors.join("; ")), + Self::Internal => write!(f, "internal: failed to parse proto JSON"), + } + } +} + +/// Validate a proto policy and project it to OPA data, without emitting +/// events. Returns the data and the L7 warnings for the caller to emit. +pub(crate) fn validated_proto_data( + proto: &ProtoSandboxPolicy, + entrypoint_pid: u32, + require_binary_identity: bool, +) -> std::result::Result<(serde_json::Value, Vec), ProtoPolicyInvalid> { + // Protobuf cannot distinguish an omitted repeated MCP version field + // from an empty one. Canonicalize before any runtime consumer reads + // the policy so both representations select the pinned default. + let proto = openshell_policy::validate_and_canonicalize_sandbox_policy(proto.clone()) + .map_err(ProtoPolicyInvalid::Policy)?; + + let ambiguities = openshell_policy::find_endpoint_ambiguities(&proto); + if !ambiguities.is_empty() { + return Err(ProtoPolicyInvalid::Ambiguous(ambiguities)); + } + + // Parse back to Value for preprocessing, then re-serialize + let mut data: serde_json::Value = + serde_json::from_str(&proto_to_opa_data_json(&proto, entrypoint_pid)) + .map_err(|_| ProtoPolicyInvalid::Internal)?; + inject_runtime_policy_data(&mut data, require_binary_identity); + normalize_endpoint_protocols(&mut data); + + // Validate BEFORE expanding presets + let (errors, warnings) = crate::l7::validate_l7_policies(&data); + if !errors.is_empty() { + return Err(ProtoPolicyInvalid::L7(errors)); + } + Ok((data, warnings)) +} + /// Convert typed proto policy fields to JSON suitable for `engine.add_data_json()`. /// /// The rego rules reference `data.*` directly, so the JSON structure has diff --git a/crates/openshell-supervisor-network/src/policy_local.rs b/crates/openshell-supervisor-network/src/policy_local.rs index 033f492aa7..e0024b25a2 100644 --- a/crates/openshell-supervisor-network/src/policy_local.rs +++ b/crates/openshell-supervisor-network/src/policy_local.rs @@ -8,7 +8,7 @@ use miette::{IntoDiagnostic, Result}; use openshell_core::grpc_client::GatewayEndpoint; use openshell_core::proposals::AgentProposals; use openshell_core::proto::{ - L7Allow, L7DenyRule, L7Rule, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, PolicyChunk, + L7Rule, McpOptions, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, PolicyChunk, SandboxPolicy as ProtoSandboxPolicy, }; use openshell_ocsf::{ @@ -1227,11 +1227,18 @@ fn network_rule_from_json( .map(|binary| NetworkBinary { path: binary.path }) .collect(); - Ok(NetworkPolicyRule { + let rule = NetworkPolicyRule { name: rule.name.unwrap_or_default(), endpoints, binaries, - }) + }; + // Reject a rule the supervisor would refuse to load after approval. + let policy = ProtoSandboxPolicy { + network_policies: HashMap::from([("proposal".to_string(), rule.clone())]), + ..Default::default() + }; + crate::opa::validated_proto_data(&policy, 0, true).map_err(|invalid| invalid.to_string())?; + Ok(rule) } fn network_endpoint_from_json( @@ -1261,6 +1268,27 @@ fn network_endpoint_from_json( if ports.is_empty() { return Err("endpoint.port or endpoint.ports is required".to_string()); } + // A more specific endpoint path can replace the protocol and enforcement + // of an operator endpoint on the same host, so only operators set it. + if !endpoint.path.is_empty() { + return Err( + "endpoint.path is not supported in proposals; ask an administrator to add it" + .to_string(), + ); + } + // `tool` is shorthand for `params.name`; when both are set the policy + // conversion keeps `params.name` and drops `tool`. + if endpoint + .rules + .iter() + .any(|rule| rule.allow.tool.is_some() && rule.allow.params.contains_key("name")) + || endpoint + .deny_rules + .iter() + .any(|rule| rule.tool.is_some() && rule.params.contains_key("name")) + { + return Err("set either `tool` or `params.name` on a rule, not both".to_string()); + } if endpoint .rules .iter() @@ -1274,34 +1302,13 @@ fn network_endpoint_from_json( .rules .into_iter() .map(|rule| L7Rule { - allow: Some(L7Allow { - method: rule.allow.method, - path: rule.allow.path, - command: rule.allow.command, - query: HashMap::new(), - // GraphQL fields default empty — agent-authored proposals from - // policy.local target REST/SQL/L4 endpoints; GraphQL operation - // matching is set on the policy server side or via direct YAML. - operation_type: String::new(), - operation_name: String::new(), - fields: Vec::new(), - params: HashMap::new(), - }), + allow: Some(openshell_policy::allow_def_to_proto(rule.allow)), }) .collect(); let deny_rules = endpoint .deny_rules .into_iter() - .map(|rule| L7DenyRule { - method: rule.method, - path: rule.path, - command: rule.command, - query: HashMap::new(), - operation_type: String::new(), - operation_name: String::new(), - fields: Vec::new(), - params: HashMap::new(), - }) + .map(openshell_policy::deny_def_to_proto) .collect(); Ok(NetworkEndpoint { @@ -1327,13 +1334,16 @@ fn network_endpoint_from_json( allow_uninspected_credentials: false, provider_credentialed: false, advisor_proposed: false, - // GraphQL persisted-query knobs and path scoping default empty — - // agent proposals don't author them today. + // GraphQL persisted-query knobs default empty — agent proposals + // don't author them today. persisted_queries: String::new(), graphql_persisted_queries: HashMap::new(), graphql_max_body_bytes: 0, json_rpc_max_body_bytes: 0, - mcp: None, + mcp: endpoint.mcp.map(|mcp| McpOptions { + versions: mcp.versions, + ..Default::default() + }), path: String::new(), credential_signing: String::new(), signing_service: String::new(), @@ -1457,7 +1467,10 @@ struct ProposalRequest { operations: Vec, } +// Proposal rule structs reject unknown keys: a dropped field could widen the +// rule the reviewer approves. #[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] struct AddNetworkRuleJson { #[serde(default, rename = "ruleName")] rule_name: Option, @@ -1465,6 +1478,7 @@ struct AddNetworkRuleJson { } #[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] struct NetworkPolicyRuleJson { #[serde(default)] name: Option, @@ -1475,9 +1489,12 @@ struct NetworkPolicyRuleJson { } #[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] struct NetworkEndpointJson { host: String, #[serde(default)] + path: String, + #[serde(default)] port: u32, #[serde(default)] ports: Vec, @@ -1494,45 +1511,36 @@ struct NetworkEndpointJson { #[serde(default)] allowed_ips: Vec, #[serde(default)] - deny_rules: Vec, + deny_rules: Vec, #[serde(default)] allow_encoded_slash: bool, + #[serde(default)] + mcp: Option, } #[derive(Debug, Deserialize)] -struct NetworkBinaryJson { - path: String, -} - -#[derive(Debug, Deserialize)] -struct L7RuleJson { - allow: L7AllowJson, +#[serde(deny_unknown_fields)] +struct McpJson { + versions: Vec, } #[derive(Debug, Deserialize)] -struct L7AllowJson { - #[serde(default)] - method: String, - #[serde(default)] +#[serde(deny_unknown_fields)] +struct NetworkBinaryJson { path: String, - #[serde(default)] - command: String, } #[derive(Debug, Deserialize)] -struct L7DenyRuleJson { - #[serde(default)] - method: String, - #[serde(default)] - path: String, - #[serde(default)] - command: String, +#[serde(deny_unknown_fields)] +struct L7RuleJson { + allow: openshell_policy_schema::L7Allow, } #[cfg(test)] mod tests { use super::*; use openshell_core::proposals::AgentProposals; + use openshell_core::proto::L7Allow; fn test_workspace_rx() -> tokio::sync::watch::Receiver { tokio::sync::watch::channel(String::new()).1 @@ -1693,6 +1701,83 @@ mod tests { ); } + #[test] + fn proposal_chunks_from_body_keeps_mcp_tool() { + let body = br#"{ + "operations": [{"addRule": {"ruleName": "wiki_mcp", "rule": { + "endpoints": [{"host": "mcp.example.org", "port": 443, + "protocol": "mcp", "enforcement": "enforce", + "mcp": {"versions": ["2025-11-25"]}, + "rules": [{"allow": {"method": "initialize"}}, + {"allow": {"method": "tools/call", "tool": "read_wiki_structure"}}], + "deny_rules": [{"method": "tools/call", "tool": "delete_*"}]}], + "binaries": [{"path": "/usr/bin/curl"}]}}}] + }"#; + + let chunks = proposal_chunks_from_body(body).unwrap(); + let endpoint = &chunks[0].proposed_rule.as_ref().unwrap().endpoints[0]; + assert_eq!( + endpoint.mcp.as_ref().unwrap().versions, + vec!["2025-11-25".to_string()] + ); + let tool_call = endpoint.rules[1].allow.as_ref().unwrap(); + assert_eq!(tool_call.method, "tools/call"); + assert_eq!(tool_call.params["name"].glob, "read_wiki_structure"); + assert_eq!(endpoint.deny_rules[0].params["name"].glob, "delete_*"); + } + + #[test] + fn proposal_chunks_from_body_rejects_rules_the_supervisor_cannot_load() { + for (endpoint, reason) in [ + ( + r#"{"host":"mcp.example.org","port":443,"path":"/mcp","protocol":"mcp"}"#, + "endpoint.path", + ), + ( + r#"{"host":"mcp.example.org","port":443,"protocol":"mcp","rules":[{"allow":{"method":"tools/call","tool":"read_x","params":{"name":"*"}}}]}"#, + "not both", + ), + ( + r#"{"host":"api.example.com","port":443,"protocol":"rest","rules":[{"allow":{"method":"POST","path":"/x","tool":"read_x"}}]}"#, + "params matching is only valid for protocol mcp", + ), + ( + r#"{"host":"mcp.example.org","port":443,"protocol":"mcp","rules":[{"allow":{"method":"tools/call","params":{"arguments":"*"}}}]}"#, + "MCP tool argument matching is not supported yet", + ), + ( + r#"{"host":"api.example.com","port":443,"protocol":"rest","mcp":{"versions":["2025-11-25"]}}"#, + "cannot configure mcp options", + ), + ] { + let body = format!( + r#"{{"operations": [{{"addRule": {{"ruleName": "bad", "rule": {{"endpoints": [{endpoint}]}}}}}}]}}"# + ); + + let error = proposal_chunks_from_body(body.as_bytes()).unwrap_err(); + assert!( + error.contains(reason), + "{endpoint}: unexpected error: {error}" + ); + } + } + + #[test] + fn proposal_chunks_from_body_rejects_unknown_rule_fields() { + for endpoint in [ + r#"{"host":"api.example.com","port":443,"credential_signing":"aws_sigv4"}"#, + r#"{"host":"api.example.com","port":443,"rules":[{"allow":{"method":"GET","unknown":"x"}}]}"#, + r#"{"host":"mcp.example.org","port":443,"protocol":"mcp","mcp":{"allow_all_known_mcp_methods":true}}"#, + ] { + let body = format!( + r#"{{"operations": [{{"addRule": {{"ruleName": "extra", "rule": {{"endpoints": [{endpoint}]}}}}}}]}}"# + ); + + let error = proposal_chunks_from_body(body.as_bytes()).unwrap_err(); + assert!(error.contains("unknown field"), "unexpected error: {error}"); + } + } + #[test] fn parse_last_query_clamps_to_max() { assert_eq!(parse_last_query("last=5"), Some(5)); diff --git a/crates/openshell-supervisor-process/src/skills/policy_advisor.md b/crates/openshell-supervisor-process/src/skills/policy_advisor.md index d8facc839b..b8774d4eb5 100644 --- a/crates/openshell-supervisor-process/src/skills/policy_advisor.md +++ b/crates/openshell-supervisor-process/src/skills/policy_advisor.md @@ -213,6 +213,8 @@ The new submission wins by structural overlap. TCP or a raw TLS tunnel, ask an administrator to add that access explicitly. - Do not include query strings, tokens, credentials, or secret values in paths. +- For an MCP server, name the tool with `tool` on the `tools/call` allow + rule. A `tools/call` allow without `tool` permits every tool. - Explain uncertainty in `intent_summary` instead of widening the rule. - If pushing with `git` fails, that is a separate L4 or protocol-specific path from GitHub REST API access. Propose it separately. diff --git a/docs/how-it-works/policies/advisor.mdx b/docs/how-it-works/policies/advisor.mdx index c77e2a77d4..dfcf66b2b0 100644 --- a/docs/how-it-works/policies/advisor.mdx +++ b/docs/how-it-works/policies/advisor.mdx @@ -215,10 +215,16 @@ and cannot be approved until validation succeeds. ## What Agents Can Propose +This stricter proposal validation targets the next minor OpenShell release. +Earlier releases ignored unsupported proposal fields. When upgrading, remove +those fields and resubmit the proposal. Put settings reserved for operators, +such as endpoint path selectors, in your policy file. + An agent proposes endpoints and binaries. An endpoint can include an access -preset, method and path allow rules, deny rules, `allowed_ips`, and -`allow_encoded_slash`. A good proposal allows one method on the narrowest path -the task needs: +preset, allow and deny rules, `allowed_ips`, `allow_encoded_slash`, and +`mcp.versions`. Allow and deny rules take the same fields as in a policy file, +including query matchers and MCP `tool` and `params`. A good proposal allows +one method on the narrowest path the task needs: ```json { @@ -258,9 +264,10 @@ the task needs: ``` Agents cannot propose `protocol: tcp` or `tls: skip`, because OpenShell cannot -inspect that traffic. The API also has no fields for GraphQL operation rules, -MCP tool rules, query matchers, credential settings, or endpoint path selectors, -and it ignores them if an agent sends them. Review the rule that +inspect that traffic. The API also has no fields for endpoint path selectors, +credential settings, GraphQL persisted queries, or MCP options other than +`versions`, and it rejects a proposal that sends a field it does not support or +a rule the sandbox could not load. Review the rule that `openshell rule get` shows, not the agent's description of it. Add rules that need those fields yourself, as described in [Manage Sandbox Policies](/how-it-works/policies/manage-policies).