diff --git a/Cargo.lock b/Cargo.lock index b412467610..2a13197c8f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4074,6 +4074,7 @@ version = "0.0.0" dependencies = [ "glob", "openshell-core", + "openshell-policy", "serde", "serde_json", "serde_yml", diff --git a/crates/openshell-policy/src/l7_validate.rs b/crates/openshell-policy/src/l7_validate.rs new file mode 100644 index 0000000000..12530b18a6 --- /dev/null +++ b/crates/openshell-policy/src/l7_validate.rs @@ -0,0 +1,388 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Shared L7 endpoint semantic validation. +//! +//! Both profile lint (`openshell-providers`) and the runtime policy +//! validator (`openshell-supervisor-network`) call +//! [`validate_l7_endpoint_semantics`] to enforce the same constraints on +//! L7 endpoint field combinations, preventing drift between lint-time +//! and runtime checks. + +/// Known L7 inspection protocols. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum L7Protocol { + Rest, + Websocket, + Graphql, + Sql, + JsonRpc, + Mcp, +} + +impl L7Protocol { + /// Parse a protocol string into a known variant. + pub fn parse(s: &str) -> Option { + match s.to_ascii_lowercase().as_str() { + "rest" => Some(Self::Rest), + "websocket" => Some(Self::Websocket), + "graphql" => Some(Self::Graphql), + "sql" => Some(Self::Sql), + "json-rpc" => Some(Self::JsonRpc), + "mcp" => Some(Self::Mcp), + _ => None, + } + } + + /// Returns `true` for protocols in the JSON-RPC family (`json-rpc`, + /// `mcp`). + pub fn is_jsonrpc_family(self) -> bool { + matches!(self, Self::JsonRpc | Self::Mcp) + } +} + +/// Fields extracted from an endpoint definition needed for L7 semantic +/// validation. Both profile lint and the runtime validator construct this +/// from their own data representation. +#[allow(clippy::struct_excessive_bools)] +pub struct L7EndpointFields<'a> { + /// Protocol string as authored (e.g. `"rest"`, `"mcp"`). Empty + /// string means no L7 protocol was specified. + pub protocol: &'a str, + + /// Access preset string (e.g. `"read-only"`, `"full"`). Empty string + /// means no access preset. + pub access: &'a str, + + /// `true` when the endpoint has a non-empty rules list. + pub has_rules: bool, + + /// `true` when the endpoint has a non-empty `deny_rules` list. + pub has_deny_rules: bool, + + /// `true` when rules are present but would deny all traffic (e.g. + /// the rules array exists but is empty, or all entries lack an allow + /// clause). + pub rules_would_deny_all: bool, + + /// Value of `mcp.allow_all_known_mcp_methods` (defaults to `false`). + pub allow_all_known_mcp_methods: bool, +} + +/// Validate the semantic consistency of an L7 endpoint's field +/// combination. +/// +/// Returns a list of error message strings. An empty list means the +/// endpoint passes validation. Messages are bare — callers prepend +/// their own location context. +pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec { + let mut errors = Vec::new(); + let protocol = ep.protocol; + let l7_protocol = if protocol.is_empty() { + None + } else { + L7Protocol::parse(protocol) + }; + let jsonrpc_family = l7_protocol.is_some_and(L7Protocol::is_jsonrpc_family); + + // 1. Unknown protocol + if !protocol.is_empty() && l7_protocol.is_none() { + errors.push(format!( + "unknown protocol '{protocol}' (expected rest, websocket, graphql, sql, json-rpc, or mcp)" + )); + } + + // 2. rules + access mutually exclusive + if ep.has_rules && !ep.access.is_empty() { + errors.push("rules and access are mutually exclusive".to_string()); + } + + // 3. JSON-RPC family cannot use access presets + if jsonrpc_family && !ep.access.is_empty() { + if protocol == "mcp" { + errors.push(format!( + "protocol {protocol} does not support access presets; \ + use rules/deny_rules or set mcp.allow_all_known_mcp_methods: true \ + for an allow-all MCP policy" + )); + } else { + errors.push(format!( + "protocol {protocol} does not support access presets; \ + use explicit rules with allow.method such as \"*\"" + )); + } + } + + // 4. json-rpc requires explicit rules + if protocol == "json-rpc" && !ep.has_rules { + errors.push(format!( + "protocol {protocol} requires explicit rules with allow.method" + )); + } + + // 5. Non-MCP protocol requires rules or access + if !protocol.is_empty() && protocol != "mcp" && !ep.has_rules && ep.access.is_empty() { + errors.push("protocol requires rules or access to define allowed traffic".to_string()); + } + + // 6. MCP requires rules when allow_all_known_mcp_methods is false + if protocol == "mcp" && !ep.has_rules && ep.access.is_empty() && !ep.allow_all_known_mcp_methods + { + errors.push( + "protocol mcp requires rules when mcp.allow_all_known_mcp_methods is false".to_string(), + ); + } + + // 7. Rules would deny all traffic + if ep.rules_would_deny_all { + errors.push( + "rules list cannot be empty (would deny all traffic). \ + Use `access: full` or remove rules." + .to_string(), + ); + } + + // 8. deny_rules require protocol + if ep.has_deny_rules && protocol.is_empty() { + errors.push("deny_rules require protocol (L7 inspection must be enabled)".to_string()); + } + + // 9. deny_rules require base allow set + if ep.has_deny_rules && protocol != "mcp" && !ep.has_rules && ep.access.is_empty() { + errors.push("deny_rules require rules or access to define the base allow set".to_string()); + } + + errors +} + +#[cfg(test)] +mod tests { + use super::*; + + fn valid_rest_endpoint() -> L7EndpointFields<'static> { + L7EndpointFields { + protocol: "rest", + access: "read-only", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + } + } + + #[test] + fn valid_endpoint_produces_no_errors() { + let errors = validate_l7_endpoint_semantics(&valid_rest_endpoint()); + assert!(errors.is_empty(), "expected no errors, got: {errors:?}"); + } + + #[test] + fn rejects_unknown_protocol() { + let ep = L7EndpointFields { + protocol: "ftp", + access: "", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!(errors.iter().any(|e| e.contains("unknown protocol"))); + } + + #[test] + fn rejects_rules_and_access_together() { + let ep = L7EndpointFields { + protocol: "rest", + access: "full", + has_rules: true, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!(errors.iter().any(|e| e.contains("mutually exclusive"))); + } + + #[test] + fn rejects_jsonrpc_with_access_presets() { + let ep = L7EndpointFields { + protocol: "json-rpc", + access: "full", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!( + errors + .iter() + .any(|e| e.contains("does not support access presets")) + ); + } + + #[test] + fn rejects_mcp_with_access_presets() { + let ep = L7EndpointFields { + protocol: "mcp", + access: "full", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!( + errors + .iter() + .any(|e| e.contains("allow_all_known_mcp_methods")) + ); + } + + #[test] + fn rejects_jsonrpc_without_rules() { + let ep = L7EndpointFields { + protocol: "json-rpc", + access: "", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!(errors.iter().any(|e| e.contains("requires explicit rules"))); + } + + #[test] + fn rejects_protocol_without_rules_or_access() { + let ep = L7EndpointFields { + protocol: "rest", + access: "", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!( + errors + .iter() + .any(|e| e.contains("protocol requires rules or access")) + ); + } + + #[test] + fn rejects_mcp_without_rules_when_allow_all_false() { + let ep = L7EndpointFields { + protocol: "mcp", + access: "", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!(errors.iter().any(|e| e.contains("mcp requires rules when"))); + } + + #[test] + fn accepts_mcp_with_allow_all_true() { + let ep = L7EndpointFields { + protocol: "mcp", + access: "", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: true, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!(errors.is_empty(), "expected no errors, got: {errors:?}"); + } + + #[test] + fn rejects_rules_that_deny_all() { + let ep = L7EndpointFields { + protocol: "rest", + access: "", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: true, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!(errors.iter().any(|e| e.contains("would deny all traffic"))); + } + + #[test] + fn rejects_deny_rules_without_protocol() { + let ep = L7EndpointFields { + protocol: "", + access: "", + has_rules: false, + has_deny_rules: true, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!( + errors + .iter() + .any(|e| e.contains("deny_rules require protocol")) + ); + } + + #[test] + fn rejects_deny_rules_without_allow_base() { + let ep = L7EndpointFields { + protocol: "rest", + access: "", + has_rules: false, + has_deny_rules: true, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!( + errors + .iter() + .any(|e| e.contains("deny_rules require rules or access")) + ); + } + + #[test] + fn no_protocol_no_errors() { + let ep = L7EndpointFields { + protocol: "", + access: "", + has_rules: false, + has_deny_rules: false, + rules_would_deny_all: false, + allow_all_known_mcp_methods: false, + }; + let errors = validate_l7_endpoint_semantics(&ep); + assert!(errors.is_empty(), "expected no errors, got: {errors:?}"); + } + + #[test] + fn l7_protocol_parse_known_variants() { + assert_eq!(L7Protocol::parse("rest"), Some(L7Protocol::Rest)); + assert_eq!(L7Protocol::parse("websocket"), Some(L7Protocol::Websocket)); + assert_eq!(L7Protocol::parse("graphql"), Some(L7Protocol::Graphql)); + assert_eq!(L7Protocol::parse("sql"), Some(L7Protocol::Sql)); + assert_eq!(L7Protocol::parse("json-rpc"), Some(L7Protocol::JsonRpc)); + assert_eq!(L7Protocol::parse("mcp"), Some(L7Protocol::Mcp)); + assert_eq!(L7Protocol::parse("unknown"), None); + assert_eq!(L7Protocol::parse(""), None); + } + + #[test] + fn l7_protocol_jsonrpc_family() { + assert!(L7Protocol::JsonRpc.is_jsonrpc_family()); + assert!(L7Protocol::Mcp.is_jsonrpc_family()); + assert!(!L7Protocol::Rest.is_jsonrpc_family()); + assert!(!L7Protocol::Websocket.is_jsonrpc_family()); + assert!(!L7Protocol::Graphql.is_jsonrpc_family()); + assert!(!L7Protocol::Sql.is_jsonrpc_family()); + } +} diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 3c72b19b32..ea838b3b74 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -10,6 +10,7 @@ //! these types, ensuring round-trip fidelity. mod compose; +mod l7_validate; mod merge; mod middleware; @@ -29,6 +30,7 @@ pub use compose::{ PROVIDER_RULE_NAME_PREFIX, ProviderPolicyLayer, compose_effective_policy, is_provider_rule_name, provider_rule_name, strip_provider_rule_names, }; +pub use l7_validate::{L7EndpointFields, L7Protocol, validate_l7_endpoint_semantics}; pub use merge::{ PolicyMergeError, PolicyMergeOp, PolicyMergeResult, PolicyMergeWarning, generated_rule_name, merge_policy, policy_covers_rule, diff --git a/crates/openshell-providers/Cargo.toml b/crates/openshell-providers/Cargo.toml index 9b294d7b71..abf6a6f11a 100644 --- a/crates/openshell-providers/Cargo.toml +++ b/crates/openshell-providers/Cargo.toml @@ -13,6 +13,7 @@ repository.workspace = true [dependencies] glob = { workspace = true } openshell-core = { path = "../openshell-core", default-features = false } +openshell-policy = { path = "../openshell-policy" } serde = { workspace = true } serde_json = { workspace = true } serde_yml = { workspace = true } diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index c3441de533..590083e602 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -13,6 +13,7 @@ use openshell_core::proto::{ ProviderProfileCredential, ProviderProfileDiscovery, }; use openshell_core::secrets::uses_reserved_revision_namespace; +use openshell_policy::{L7EndpointFields, validate_l7_endpoint_semantics}; use serde::ser::SerializeStruct; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; use std::collections::{HashMap, HashSet}; @@ -1616,6 +1617,28 @@ pub fn validate_profile_set( format!("invalid endpoint '{}:{}'", endpoint.host, endpoint.port), )); } + + let l7_fields = L7EndpointFields { + protocol: &endpoint.protocol, + access: &endpoint.access, + has_rules: !endpoint.rules.is_empty(), + has_deny_rules: !endpoint.deny_rules.is_empty(), + rules_would_deny_all: !endpoint.rules.is_empty() + && endpoint.rules.iter().all(|r| r.allow.is_none()), + allow_all_known_mcp_methods: endpoint + .mcp + .as_ref() + .and_then(|opts| opts.allow_all_known_mcp_methods) + .unwrap_or(false), + }; + for msg in validate_l7_endpoint_semantics(&l7_fields) { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + format!("endpoints[{index}]"), + msg, + )); + } } for (index, binary) in profile.binaries.iter().enumerate() { @@ -2764,12 +2787,23 @@ id: advanced display_name: Advanced category: other endpoints: + - host: graphql.example.com + port: 443 + protocol: graphql + access: read-only + persisted_queries: allow_registered + graphql_persisted_queries: + hash-a: + operation_type: query + operation_name: Viewer + fields: [viewer] + graphql_max_body_bytes: 131072 + path: /graphql - host: api.example.com ports: [443, 8443] protocol: rest tls: terminate enforcement: enforce - access: read-only rules: - allow: method: GET @@ -2782,14 +2816,6 @@ endpoints: - method: POST path: /admin/** allow_encoded_slash: true - persisted_queries: allow_registered - graphql_persisted_queries: - hash-a: - operation_type: query - operation_name: Viewer - fields: [viewer] - graphql_max_body_bytes: 131072 - path: /graphql binaries: - path: /usr/bin/custom harness: true @@ -2803,39 +2829,44 @@ binaries: ); let proto = profile.to_proto(); - let endpoint = proto.endpoints.first().expect("endpoint should exist"); - assert_eq!(endpoint.port, 0); - assert_eq!(endpoint.ports, vec![443, 8443]); - assert_eq!(endpoint.tls, "terminate"); - assert_eq!(endpoint.allowed_ips, vec!["10.0.0.0/24"]); - assert!(endpoint.allow_encoded_slash); - assert_eq!(endpoint.persisted_queries, "allow_registered"); - assert_eq!(endpoint.graphql_max_body_bytes, 131_072); - assert_eq!(endpoint.path, "/graphql"); + + let graphql_ep = &proto.endpoints[0]; + assert_eq!(graphql_ep.access, "read-only"); + assert_eq!(graphql_ep.persisted_queries, "allow_registered"); + assert_eq!(graphql_ep.graphql_max_body_bytes, 131_072); + assert_eq!(graphql_ep.path, "/graphql"); + assert_eq!( + graphql_ep + .graphql_persisted_queries + .get("hash-a") + .map(|operation| operation.operation_name.as_str()), + Some("Viewer") + ); + + let rest_ep = &proto.endpoints[1]; + assert_eq!(rest_ep.port, 0); + assert_eq!(rest_ep.ports, vec![443, 8443]); + assert_eq!(rest_ep.tls, "terminate"); + assert_eq!(rest_ep.allowed_ips, vec!["10.0.0.0/24"]); + assert!(rest_ep.allow_encoded_slash); assert_eq!( - endpoint + rest_ep .rules .first() .and_then(|rule| rule.allow.as_ref()) .map(|allow| allow.method.as_str()), Some("GET") ); - assert_eq!(endpoint.deny_rules[0].method, "POST"); - assert_eq!( - endpoint - .graphql_persisted_queries - .get("hash-a") - .map(|operation| operation.operation_name.as_str()), - Some("Viewer") - ); + assert_eq!(rest_ep.deny_rules[0].method, "POST"); assert!(proto.binaries[0].harness); let reparsed = parse_profile_yaml(&profile_to_yaml(&profile).expect("serialize YAML")) .expect("serialized profile should parse"); let reprotoo = reparsed.to_proto(); - assert_eq!(reprotoo.endpoints[0].rules.len(), 1); - assert_eq!(reprotoo.endpoints[0].deny_rules.len(), 1); - assert_eq!(reprotoo.endpoints[0].ports, vec![443, 8443]); + assert_eq!(reprotoo.endpoints[0].access, "read-only"); + assert_eq!(reprotoo.endpoints[1].rules.len(), 1); + assert_eq!(reprotoo.endpoints[1].deny_rules.len(), 1); + assert_eq!(reprotoo.endpoints[1].ports, vec![443, 8443]); assert!(reprotoo.binaries[0].harness); } @@ -3431,4 +3462,173 @@ credentials: "unexpected diagnostics: {diagnostics:?}" ); } + + // -- L7 endpoint semantic validation (shared with runtime) ---------------- + + #[test] + fn validate_rejects_protocol_without_rules_or_access() { + let profile = parse_profile_yaml( + r" +id: opencode-openrouter +display_name: OpenCode (OpenRouter) +credentials: + - name: api_key + env_vars: [OPENROUTER_API_KEY] + auth_style: bearer + header_name: authorization +discovery: + credentials: [api_key] +endpoints: + - host: openrouter.ai + port: 443 + protocol: rest + enforcement: enforce +binaries: + - /usr/bin/opencode +", + ) + .expect("profile should parse"); + + let diagnostics = validate_profile_set(&[("profile.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("protocol requires rules or access")), + "expected lint to reject protocol without rules or access, got: {diagnostics:?}" + ); + } + + #[test] + fn validate_accepts_protocol_with_access() { + let profile = parse_profile_yaml( + r" +id: valid-rest +display_name: Valid REST +credentials: + - name: api_key + env_vars: [API_KEY] + auth_style: bearer + header_name: authorization +discovery: + credentials: [api_key] +endpoints: + - host: api.example.com + port: 443 + protocol: rest + access: read-write +binaries: + - /usr/bin/app +", + ) + .expect("profile should parse"); + + let diagnostics = validate_profile_set(&[("profile.yaml".to_string(), profile)]); + let errors: Vec<_> = diagnostics + .iter() + .filter(|d| d.severity == "error") + .collect(); + assert!(errors.is_empty(), "unexpected errors: {errors:?}"); + } + + #[test] + fn validate_rejects_unknown_protocol() { + let profile = parse_profile_yaml( + r" +id: bad-protocol +display_name: Bad Protocol +credentials: + - name: api_key + env_vars: [API_KEY] + auth_style: bearer + header_name: authorization +discovery: + credentials: [api_key] +endpoints: + - host: api.example.com + port: 443 + protocol: ftp +binaries: + - /usr/bin/app +", + ) + .expect("profile should parse"); + + let diagnostics = validate_profile_set(&[("profile.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("unknown protocol")), + "expected lint to reject unknown protocol, got: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_rules_and_access_together() { + let profile = parse_profile_yaml( + r" +id: both-rules-access +display_name: Both +credentials: + - name: api_key + env_vars: [API_KEY] + auth_style: bearer + header_name: authorization +discovery: + credentials: [api_key] +endpoints: + - host: api.example.com + port: 443 + protocol: rest + access: full + rules: + - allow: + method: GET + path: /api/** +binaries: + - /usr/bin/app +", + ) + .expect("profile should parse"); + + let diagnostics = validate_profile_set(&[("profile.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("mutually exclusive")), + "expected lint to reject rules + access, got: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_deny_rules_without_protocol() { + let profile = parse_profile_yaml( + r" +id: deny-no-protocol +display_name: Deny No Protocol +credentials: + - name: api_key + env_vars: [API_KEY] + auth_style: bearer + header_name: authorization +discovery: + credentials: [api_key] +endpoints: + - host: api.example.com + port: 443 + deny_rules: + - method: POST +binaries: + - /usr/bin/app +", + ) + .expect("profile should parse"); + + let diagnostics = validate_profile_set(&[("profile.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("deny_rules require protocol")), + "expected lint to reject deny_rules without protocol, got: {diagnostics:?}" + ); + } } diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 59774bbcc8..b9332630bc 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -8113,6 +8113,7 @@ mod tests { host: "api.github.com".to_string(), port: 443, protocol: "rest".to_string(), + access: "full".to_string(), deny_rules: vec![L7DenyRule { method: "DELETE".to_string(), path: "/repos/*".to_string(), diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index cae751b42a..337b6f9200 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2714,6 +2714,7 @@ mod tests { port, path: path.to_string(), protocol: "rest".to_string(), + access: "full".to_string(), ..Default::default() }]; handle_import_provider_profiles( @@ -2842,6 +2843,7 @@ mod tests { port: 443, path: "/v1/**".to_string(), protocol: "rest".to_string(), + access: "full".to_string(), ..Default::default() }]; let response = handle_import_provider_profiles( @@ -3167,6 +3169,7 @@ mod tests { port: 443, path: "/v1/**".to_string(), protocol: "rest".to_string(), + access: "full".to_string(), ..Default::default() }]; let response = handle_update_provider_profiles( diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index c9330a3de1..d712cf982a 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -21,6 +21,8 @@ pub mod tls; pub(crate) mod token_grant_injection; pub(crate) mod websocket; +use openshell_policy::{L7EndpointFields, validate_l7_endpoint_semantics}; + /// Application-layer protocol for L7 inspection. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum L7Protocol { @@ -1031,40 +1033,31 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< )); } - // rules + access mutual exclusion - if has_rules && !access.is_empty() { - errors.push(format!("{loc}: rules and access are mutually exclusive")); - } - - if jsonrpc_family && !access.is_empty() { - if protocol == "mcp" { - errors.push(format!( - "{loc}: protocol {protocol} does not support access presets; use rules/deny_rules or set mcp.allow_all_known_mcp_methods: true for an allow-all MCP policy" - )); - } else { - errors.push(format!( - "{loc}: protocol {protocol} does not support access presets; use explicit rules with allow.method such as \"*\"" - )); - } - } - - if protocol == "json-rpc" && !has_rules { - errors.push(format!( - "{loc}: protocol {protocol} requires explicit rules with allow.method" - )); - } - - // protocol requires rules or access - if !protocol.is_empty() && protocol != "mcp" && !has_rules && access.is_empty() { - errors.push(format!( - "{loc}: protocol requires rules or access to define allowed traffic" - )); - } - - if !protocol.is_empty() && l7_protocol.is_none() { - errors.push(format!( - "{loc}: unknown protocol '{protocol}' (expected rest, websocket, graphql, sql, json-rpc, or mcp)" - )); + // L7 endpoint semantic validation (shared with profile lint). + // Computed lazily: has_deny_rules and rules_would_deny_all are + // needed here but also referenced by per-rule checks below. + let has_deny_rules = ep + .get("deny_rules") + .and_then(|v| v.as_array()) + .is_some_and(|a| !a.is_empty()); + let rules_would_deny_all = ep + .get("rules") + .and_then(|v| v.as_array()) + .is_some_and(Vec::is_empty); + let mcp_allow_all_known_mcp_methods = ep + .get("mcp_allow_all_known_mcp_methods") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + let l7_fields = L7EndpointFields { + protocol, + access, + has_rules, + has_deny_rules, + rules_would_deny_all, + allow_all_known_mcp_methods: mcp_allow_all_known_mcp_methods, + }; + for msg in validate_l7_endpoint_semantics(&l7_fields) { + errors.push(format!("{loc}: {msg}")); } if let Some(mode) = ep.get("persisted_queries").and_then(|v| v.as_str()) @@ -1154,20 +1147,6 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< .get("mcp_strict_tool_names") .and_then(serde_json::Value::as_bool) .unwrap_or(true); - let mcp_allow_all_known_mcp_methods = ep - .get("mcp_allow_all_known_mcp_methods") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false); - if protocol == "mcp" - && !has_rules - && access.is_empty() - && !mcp_allow_all_known_mcp_methods - { - errors.push(format!( - "{loc}: protocol mcp requires rules when mcp.allow_all_known_mcp_methods is false" - )); - } - if ep .get("websocket_credential_rewrite") .and_then(serde_json::Value::as_bool) @@ -1225,41 +1204,13 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< )); } - // rules with empty list - if ep - .get("rules") - .and_then(|v| v.as_array()) - .is_some_and(Vec::is_empty) - { - errors.push(format!( - "{loc}: rules list cannot be empty (would deny all traffic). Use `access: full` or remove rules." - )); - } - // port 443 + rest + tls: skip — L7 won't work (already handled above) // The old warning about missing `tls: terminate` is no longer needed // because TLS termination is now automatic. - // Validate deny_rules - let has_deny_rules = ep - .get("deny_rules") - .and_then(|v| v.as_array()) - .is_some_and(|a| !a.is_empty()); + // Per-rule deny_rules validation (semantic checks handled by + // shared validator above). if has_deny_rules { - // deny_rules require L7 inspection - if protocol.is_empty() { - errors.push(format!( - "{loc}: deny_rules require protocol (L7 inspection must be enabled)" - )); - } - - // deny_rules require some allow base (access or rules) - if protocol != "mcp" && !has_rules && access.is_empty() { - errors.push(format!( - "{loc}: deny_rules require rules or access to define the base allow set" - )); - } - let has_mcp_tool_allow_selectors = protocol == "mcp" && mcp_endpoint_has_tool_allow_selectors(ep);