fix(policy): reject unsupported tls endpoint values

Signed-off-by: Yuedong Wu <dwcn22@outlook.com>

Author: lunarwhite

architecture/security-policy.md

@@ -123,6 +123,11 @@ stream, but it does not inspect TLS SNI, HTTP `Host`, or another protocol-level
 destination. Compatible shared infrastructure can therefore let a client
 select another tenant, virtual host, or service behind the approved front door.
 
+An endpoint's `tls` field is either omitted, meaning auto-detect and terminate
+for inspection, or `skip`. Every other value fails shared L7 endpoint
+validation, so a policy cannot name a transport mode the proxy does not
+implement.
+
 ## Credentialed Endpoints
 
 OpenShell keeps provider credentials on paths it can inspect or rewrite by

crates/openshell-cli/tests/provider_commands_integration.rs

@@ -2604,7 +2604,6 @@ endpoints:
   - host: api.advanced.example
     ports: [443, 8443]
     protocol: rest
-    tls: terminate
     enforcement: enforce
     rules:
       - allow:

crates/openshell-driver-mxc/tests/wxc_exec_real.rs

@@ -732,7 +732,6 @@ async fn pc_https_egress_reads_injected_ca_bundle() {
                     host: "example.com".to_string(),
                     ports: vec![443],
                     protocol: "rest".to_string(),
-                    tls: "terminate".to_string(),
                     enforcement: "enforce".to_string(),
                     access: "read-only".to_string(),
                     ..Default::default()

crates/openshell-policy/src/l7_validate.rs

@@ -49,6 +49,23 @@ pub fn is_explicit_tcp_protocol(protocol: &str) -> bool {
     protocol.eq_ignore_ascii_case("tcp")
 }
 
+/// `skip` is deliberately not offered as an alternative: it is a posture
+/// downgrade, not a fix.
+const TLS_MODE_REMEDIATION: &str = "remove the tls field to keep automatic TLS termination";
+
+/// Reject `tls` values outside the supported set.
+///
+/// Omitted means auto-detect and terminate for inspection; `skip` opts out.
+pub fn validate_tls_mode(tls: &str) -> Option<String> {
+    if tls.is_empty() || tls.eq_ignore_ascii_case("skip") {
+        return None;
+    }
+
+    Some(format!(
+        "unsupported tls value '{tls}'; {TLS_MODE_REMEDIATION}"
+    ))
+}
+
 /// Reject transport choices that an in-sandbox agent must not grant itself.
 ///
 /// An omitted protocol remains allowed: it uses the established explicit
@@ -101,12 +118,33 @@ mod agent_transport_tests {
     #[test]
     fn agent_cannot_request_native_tcp_or_skip_tls_inspection() {
         assert!(agent_authored_transport_rejection("tcp", "").is_some());
-        assert!(agent_authored_transport_rejection("TCP", "terminate").is_some());
+        assert!(agent_authored_transport_rejection("TCP", "").is_some());
         assert!(agent_authored_transport_rejection("", "skip").is_some());
         assert!(agent_authored_transport_rejection("rest", "SKIP").is_some());
     }
 }
 
+#[cfg(test)]
+mod tls_mode_tests {
+    use super::validate_tls_mode;
+
+    #[test]
+    fn omitted_and_skip_are_accepted() {
+        for tls in ["", "skip"] {
+            assert_eq!(validate_tls_mode(tls), None, "tls: {tls:?}");
+        }
+    }
+
+    #[test]
+    fn every_other_value_is_rejected() {
+        for tls in ["terminate", "passthrough", "bogus"] {
+            let error = validate_tls_mode(tls).expect("must be rejected");
+            assert!(error.contains(&format!("unsupported tls value '{tls}'")));
+            assert!(error.contains("remove the tls field"));
+        }
+    }
+}
+
 /// 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.
@@ -120,6 +158,9 @@ pub struct L7EndpointFields<'a> {
     /// means no access preset.
     pub access: &'a str,
 
+    /// TLS handling as authored. Empty string means the default.
+    pub tls: &'a str,
+
     /// `true` when the endpoint has a non-empty rules list.
     pub has_rules: bool,
 
@@ -134,8 +175,8 @@ pub struct L7EndpointFields<'a> {
     pub allow_all_known_mcp_methods: bool,
 }
 
-/// Validate the semantic consistency of an L7 endpoint's field
-/// combination.
+/// Validate an L7 endpoint's individual field values and their semantic
+/// consistency with each other.
 ///
 /// Returns a list of error message strings. An empty list means the
 /// endpoint passes validation. Messages are bare — callers prepend
@@ -226,6 +267,11 @@ pub fn validate_l7_endpoint_semantics(ep: &L7EndpointFields<'_>) -> Vec<String>
         errors.push("deny_rules require rules or access to define the base allow set".to_string());
     }
 
+    // 10. Unsupported tls value.
+    if let Some(reason) = validate_tls_mode(ep.tls) {
+        errors.push(reason);
+    }
+
     errors
 }
 
@@ -237,6 +283,7 @@ mod tests {
         L7EndpointFields {
             protocol: "rest",
             access: "read-only",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -255,6 +302,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "ftp",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -269,6 +317,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "rest",
             access: "full",
+            tls: "",
             has_rules: true,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -283,6 +332,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "json-rpc",
             access: "full",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -301,6 +351,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "mcp",
             access: "full",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -319,6 +370,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "json-rpc",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -333,6 +385,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "rest",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -351,6 +404,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "mcp",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -365,6 +419,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "mcp",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -379,6 +434,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "rest",
             access: "",
+            tls: "",
             has_rules: true,
             has_deny_rules: false,
             rules_would_deny_all: true,
@@ -393,6 +449,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "mcp",
             access: "",
+            tls: "",
             has_rules: true,
             has_deny_rules: false,
             rules_would_deny_all: true,
@@ -410,6 +467,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: true,
             rules_would_deny_all: false,
@@ -428,6 +486,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "rest",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: true,
             rules_would_deny_all: false,
@@ -446,6 +505,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -460,6 +520,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "tcp",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -476,6 +537,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "tcp",
             access: "full",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -520,6 +582,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "json-rpc",
             access: "full",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,
@@ -541,6 +604,7 @@ mod tests {
         let ep = L7EndpointFields {
             protocol: "json-rpc",
             access: "",
+            tls: "",
             has_rules: false,
             has_deny_rules: false,
             rules_would_deny_all: false,

crates/openshell-policy/src/lib.rs

@@ -38,7 +38,7 @@ pub use compose::{
 };
 pub use l7_validate::{
     L7EndpointFields, L7Protocol, agent_authored_transport_rejection,
-    validate_explicit_tcp_additional_fields, validate_l7_endpoint_semantics,
+    validate_explicit_tcp_additional_fields, validate_l7_endpoint_semantics, validate_tls_mode,
 };
 pub use merge::{
     PolicyMergeError, PolicyMergeOp, PolicyMergeResult, PolicyMergeWarning,
@@ -1513,6 +1513,7 @@ fn validate_sandbox_policy_with_mcp_presence(
             let fields = L7EndpointFields {
                 protocol: &ep.protocol,
                 access: &ep.access,
+                tls: &ep.tls,
                 has_rules: !ep.rules.is_empty(),
                 has_deny_rules: !ep.deny_rules.is_empty(),
                 rules_would_deny_all,
@@ -3871,6 +3872,35 @@ network_policies:
         assert!(validate_sandbox_policy(&policy).is_ok());
     }
 
+    #[test]
+    fn validate_rejects_unknown_tls_mode() {
+        let mut policy = restrictive_default_policy();
+        policy.network_policies.insert(
+            "api".into(),
+            NetworkPolicyRule {
+                name: "api".into(),
+                endpoints: vec![NetworkEndpoint {
+                    host: "api.example.com".into(),
+                    port: 443,
+                    tls: "terminate".into(),
+                    ..Default::default()
+                }],
+                ..Default::default()
+            },
+        );
+
+        let violations =
+            validate_sandbox_policy(&policy).expect_err("unsupported tls value must be rejected");
+        assert!(
+            violations.iter().any(|v| matches!(
+                v,
+                PolicyViolation::InvalidL7Endpoint { reason, .. }
+                    if reason.contains("unsupported tls value 'terminate'")
+            )),
+            "should be rejected: {violations:?}"
+        );
+    }
+
     #[test]
     fn validate_rejects_sigv4_no_body_without_signing_service() {
         let mut policy = restrictive_default_policy();

crates/openshell-policy/src/merge.rs

@@ -697,7 +697,7 @@ fn endpoint_attributes_cover(loaded: &NetworkEndpoint, proposed: &NetworkEndpoin
     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) {
+    if !proposed.tls.is_empty() && loaded.tls != proposed.tls {
         return false;
     }
     if !proposed.enforcement.is_empty()
@@ -801,13 +801,6 @@ fn protocols_match(left: &str, right: &str) -> bool {
     }
 }
 
-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 }
 }
@@ -3537,7 +3530,6 @@ mod tests {
         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",
@@ -3546,13 +3538,17 @@ mod tests {
         );
         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));
+        // Coverage validates both sides, so a legacy spelling fails closed
+        // rather than comparing equal to the automatic default.
+        for legacy in ["terminate", "passthrough"] {
+            explicit_defaults.tls = legacy.to_string();
+            let legacy_rule = rule_with_authorizations(
+                "proposed",
+                vec![explicit_defaults.clone()],
+                &["/usr/bin/client"],
+            );
+            assert!(!policy_covers_rule(&loaded, &legacy_rule), "tls: {legacy}");
+        }
 
         explicit_defaults.tls = "skip".to_string();
         let skip_tls = rule_with_authorizations(