fix(policy)!: reject removed tls endpoint values

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

Author: lunarwhite

architecture/security-policy.md

@@ -242,8 +242,12 @@ higher specificity rank deterministically overrides broader request-processing
 metadata. Equally specific overlapping endpoints must agree.
 
 Endpoint `tls`, `enforcement`, and `access` use protobuf enums and retain their
-named YAML spellings. `protocol` remains a string so the supported protocol set
-can evolve, but every ingress validates it before persistence or activation.
+named YAML spellings. `tls` admits only an omitted value, meaning auto-detect
+and terminate for inspection, or `skip`; the deprecated `terminate` and
+`passthrough` enum members stay representable so a stored policy carrying one
+can be named in a diagnostic, but neither validates. `protocol` remains a
+string so the supported protocol set can evolve, but every ingress validates it
+before persistence or activation.
 The supervisor also refuses unknown enum numbers and protocol values
 defensively; an unrecognized enforcement value never falls back to audit.
 

crates/openshell-cli/tests/provider_commands_integration.rs

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

crates/openshell-policy/src/l7_validate.rs

@@ -68,10 +68,16 @@ pub fn network_access_preset_to_str(value: i32) -> Option<&'static str> {
     }
 }
 
+fn unknown_tls_value(tls: &str) -> Option<String> {
+    (!matches!(tls, "" | "skip"))
+        .then(|| format!("unknown tls value '{tls}' (expected skip or omitted)"))
+}
+
 pub fn validate_endpoint_mode_values(tls: i32, enforcement: i32, access: i32) -> Vec<String> {
     let mut errors = Vec::new();
-    if network_tls_mode_to_str(tls).is_none() {
-        errors.push(format!("unknown tls enum value {tls}"));
+    match network_tls_mode_to_str(tls) {
+        Some(value) => errors.extend(unknown_tls_value(value)),
+        None => errors.push(format!("unknown tls enum value {tls}")),
     }
     if network_enforcement_mode_to_str(enforcement).is_none() {
         errors.push(format!("unknown enforcement enum value {enforcement}"));
@@ -174,7 +180,7 @@ 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());
     }
@@ -185,11 +191,7 @@ mod agent_transport_tests {
 pub fn validate_endpoint_modes(tls: &str, enforcement: &str, access: &str) -> Vec<String> {
     let mut errors = Vec::new();
 
-    if !matches!(tls, "" | "skip" | "terminate" | "passthrough") {
-        errors.push(format!(
-            "unknown tls value '{tls}' (expected skip, terminate, or passthrough)"
-        ));
-    }
+    errors.extend(unknown_tls_value(tls));
     if !matches!(enforcement, "" | "enforce" | "audit") {
         errors.push(format!(
             "unknown enforcement value '{enforcement}' (expected enforce or audit)"
@@ -357,9 +359,26 @@ mod tests {
         assert!(errors[2].contains("unknown access value 'read-wirte'"));
     }
 
+    #[test]
+    #[allow(deprecated)]
+    fn endpoint_modes_reject_removed_tls_spellings() {
+        for (spelling, value) in [
+            ("terminate", NetworkTlsMode::Terminate),
+            ("passthrough", NetworkTlsMode::Passthrough),
+        ] {
+            let errors = validate_endpoint_modes(spelling, "", "");
+            assert_eq!(errors.len(), 1, "{spelling}: {errors:?}");
+            assert!(errors[0].contains(&format!("unknown tls value '{spelling}'")));
+
+            let errors = validate_endpoint_mode_values(value as i32, 0, 0);
+            assert_eq!(errors.len(), 1, "{spelling}: {errors:?}");
+            assert!(errors[0].contains(&format!("unknown tls value '{spelling}'")));
+        }
+    }
+
     #[test]
     fn endpoint_modes_accept_documented_values_and_defaults() {
-        for tls in ["", "skip", "terminate", "passthrough"] {
+        for tls in ["", "skip"] {
             for enforcement in ["", "enforce", "audit"] {
                 for access in ["", "read-only", "read-write", "full"] {
                     assert!(validate_endpoint_modes(tls, enforcement, access).is_empty());

crates/openshell-policy/src/merge.rs

@@ -833,9 +833,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 != NetworkTlsMode::Unspecified as i32
-        && effective_tls(loaded.tls) != effective_tls(proposed.tls)
-    {
+    if proposed.tls != NetworkTlsMode::Unspecified as i32 && loaded.tls != proposed.tls {
         return false;
     }
     if proposed.enforcement != NetworkEnforcementMode::Unspecified as i32
@@ -940,19 +938,6 @@ fn protocols_match(left: &str, right: &str) -> bool {
     }
 }
 
-#[allow(deprecated)]
-fn effective_tls(value: i32) -> i32 {
-    match value {
-        value
-            if value == NetworkTlsMode::Terminate as i32
-                || value == NetworkTlsMode::Passthrough as i32 =>
-        {
-            NetworkTlsMode::Unspecified as i32
-        }
-        value => value,
-    }
-}
-
 fn effective_enforcement(value: i32) -> i32 {
     if value == NetworkEnforcementMode::Unspecified as i32 {
         NetworkEnforcementMode::Audit as i32
@@ -3783,7 +3768,6 @@ mod tests {
         assert!(!policy_covers_rule(&loaded, &different_body));
 
         let mut explicit_defaults = loaded_endpoint;
-        explicit_defaults.tls = 3; // deprecated passthrough compatibility value
         explicit_defaults.enforcement = NetworkEnforcementMode::Audit as i32;
         let runtime_defaults = rule_with_authorizations(
             "proposed",
@@ -3792,13 +3776,15 @@ mod tests {
         );
         assert!(policy_covers_rule(&loaded, &runtime_defaults));
 
-        explicit_defaults.tls = 2; // deprecated terminate compatibility value
-        let legacy_terminate = rule_with_authorizations(
-            "proposed",
-            vec![explicit_defaults.clone()],
-            &["/usr/bin/client"],
-        );
-        assert!(policy_covers_rule(&loaded, &legacy_terminate));
+        for legacy in [2, 3] {
+            explicit_defaults.tls = legacy;
+            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 = NetworkTlsMode::Skip as i32;
         let skip_tls = rule_with_authorizations(

crates/openshell-prover/src/containment.rs

@@ -1596,7 +1596,7 @@ fn validate_supported_endpoint_extensions(
     context: &str,
     endpoint: &Endpoint,
 ) -> Result<(), UnsupportedFeature> {
-    if !matches!(endpoint.tls.as_str(), "" | "terminate" | "passthrough")
+    if !endpoint.tls.is_empty()
         || endpoint.allow_encoded_slash
         || endpoint.websocket_credential_rewrite
         || endpoint.request_body_credential_rewrite
@@ -3184,17 +3184,20 @@ network_policies:
     }
 
     #[test]
-    fn deprecated_tls_spelling_does_not_change_authority() {
-        let boundary = parse(
-            "version: 1\nnetwork_policies:\n  n:\n    endpoints:\n      - host: api.example.com\n        port: 443\n        protocol: rest\n        tls: terminate\n        enforcement: enforce\n        access: read-only\n    binaries: [{ path: /usr/bin/curl }]\n",
-        );
-        let candidate = parse(
-            "version: 1\nnetwork_policies:\n  n:\n    endpoints:\n      - host: api.example.com\n        port: 443\n        protocol: rest\n        enforcement: enforce\n        rules:\n          - allow: { method: GET, path: '/v1/**' }\n    binaries: [{ path: /usr/bin/curl }]\n",
-        );
-        assert!(matches!(
-            check_within_boundary(&boundary, &candidate, options()),
-            CheckResult::Within(_)
-        ));
+    fn removed_tls_spelling_is_outside_the_authority_model() {
+        for tls in ["terminate", "passthrough"] {
+            let policy = parse(&format!(
+                "version: 1\nnetwork_policies:\n  n:\n    endpoints:\n      - host: api.example.com\n        port: 443\n        protocol: rest\n        tls: {tls}\n        enforcement: enforce\n        access: read-only\n    binaries: [{{ path: /usr/bin/curl }}]\n"
+            ));
+            assert!(
+                matches!(
+                    check_within_boundary(&policy, &policy, options()),
+                    CheckResult::Unsupported(ref evidence)
+                        if evidence.reason().contains("outside the initial model")
+                ),
+                "tls: {tls}"
+            );
+        }
     }
 
     #[test]

crates/openshell-providers/src/profiles.rs

@@ -5516,7 +5516,6 @@ endpoints:
   - host: api.example.com
     ports: [443, 8443]
     protocol: rest
-    tls: terminate
     enforcement: enforce
     rules:
       - allow:
@@ -5565,7 +5564,7 @@ binaries:
         assert_eq!(rest_ep.ports, vec![443, 8443]);
         assert_eq!(
             rest_ep.tls,
-            openshell_core::proto::NetworkTlsMode::Terminate as i32
+            openshell_core::proto::NetworkTlsMode::Unspecified as i32
         );
         assert_eq!(rest_ep.allowed_ips, vec!["10.0.0.0/24"]);
         assert!(rest_ep.allow_encoded_slash);

crates/openshell-server/src/grpc/policy.rs

@@ -10004,14 +10004,12 @@ mod tests {
         }
     }
 
-    #[allow(deprecated)]
     fn l7_scope_policy() -> ProtoSandboxPolicy {
         let endpoint = NetworkEndpoint {
             host: "api.example.com".to_string(),
             port: 443,
             ports: vec![443, 8443],
             protocol: "rest".to_string(),
-            tls: openshell_core::proto::NetworkTlsMode::Terminate as i32,
             access: openshell_core::proto::NetworkAccessPreset::ReadOnly as i32,
             ..Default::default()
         };
@@ -10967,7 +10965,6 @@ mod tests {
         let mut policy = test_policy_with_rule("aws", host);
         let endpoint = &mut policy.network_policies.get_mut("aws").unwrap().endpoints[0];
         endpoint.protocol = "rest".to_string();
-        endpoint.tls = 2;
         endpoint.access = openshell_core::proto::NetworkAccessPreset::Full as i32;
         endpoint.credential_signing = "sigv4".to_string();
         endpoint.signing_service = "s3".to_string();
@@ -13032,7 +13029,6 @@ mod tests {
             .endpoints[0];
         bound_endpoint.protocol = "rest".to_string();
         bound_endpoint.access = openshell_core::proto::NetworkAccessPreset::Full as i32;
-        bound_endpoint.tls = 2;
         openshell_policy::ensure_sandbox_process_identity(&mut policy);
         state
             .store

crates/openshell-supervisor-network/src/l7/mod.rs

@@ -307,30 +307,6 @@ pub fn parse_l7_config(val: &regorus::Value) -> Option<L7EndpointConfig> {
 
     let tls = match tls_value.as_str() {
         "skip" => TlsMode::Skip,
-        "terminate" => {
-            let event = openshell_ocsf::NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx())
-                .activity(openshell_ocsf::ActivityId::Other)
-                .severity(openshell_ocsf::SeverityId::Medium)
-                .message(
-                    "'tls: terminate' is deprecated; TLS termination is now automatic. \
-                     Use 'tls: skip' to explicitly disable. This field will be removed in a future version.",
-                )
-                .build();
-            openshell_ocsf::ocsf_emit!(event);
-            TlsMode::Auto
-        }
-        "passthrough" => {
-            let event = openshell_ocsf::NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx())
-                .activity(openshell_ocsf::ActivityId::Other)
-                .severity(openshell_ocsf::SeverityId::Medium)
-                .message(
-                    "'tls: passthrough' is deprecated; TLS termination is now automatic. \
-                     Use 'tls: skip' to explicitly disable. This field will be removed in a future version.",
-                )
-                .build();
-            openshell_ocsf::ocsf_emit!(event);
-            TlsMode::Auto
-        }
         "" => TlsMode::Auto,
         _ => unreachable!("endpoint modes were validated above"),
     };
@@ -470,7 +446,6 @@ pub fn endpoint_path_matches(pattern: &str, path: &str) -> bool {
 pub fn parse_tls_mode(val: &regorus::Value) -> TlsMode {
     match get_object_str(val, "tls").as_deref() {
         Some("skip") => TlsMode::Skip,
-        // "terminate" and "passthrough" are deprecated aliases (logged by parse_l7_config); fall through to Auto.
         _ => TlsMode::Auto,
     }
 }
@@ -1273,6 +1248,12 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, Vec<
             );
             let loc = format!("{name}.endpoints[{i}]");
 
+            errors.extend(
+                validate_endpoint_modes(tls, enforcement, access)
+                    .into_iter()
+                    .map(|reason| format!("{loc}: {reason}")),
+            );
+
             if protocol == "mcp" {
                 if host.trim().is_empty() {
                     errors.push(format!(
@@ -1489,13 +1470,6 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, Vec<
                 }
             }
 
-            // Deprecated tls values: warn but don't error
-            if tls == "terminate" || tls == "passthrough" {
-                warnings.push(format!(
-                    "{loc}: 'tls: {tls}' is deprecated; TLS termination is now automatic. Use 'tls: skip' to disable."
-                ));
-            }
-
             // tls: skip with L7 on port 443 won't work
             if tls == "skip" && !protocol.is_empty() && ports.contains(&443) {
                 warnings.push(format!(
@@ -1510,10 +1484,6 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, Vec<
                 ));
             }
 
-            // 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.
-
             // Per-rule deny_rules validation (semantic checks handled by
             // shared validator above).
             if has_deny_rules {
@@ -1953,12 +1923,11 @@ mod tests {
     #[test]
     fn parse_l7_config_rest_enforce() {
         let val = regorus::Value::from_json_str(
-            r#"{"protocol": "rest", "tls": "terminate", "enforcement": "enforce", "host": "api.example.com", "port": 443}"#,
+            r#"{"protocol": "rest", "enforcement": "enforce", "host": "api.example.com", "port": 443}"#,
         )
         .unwrap();
         let config = parse_l7_config(&val).unwrap();
         assert_eq!(config.protocol, L7Protocol::Rest);
-        // "terminate" is deprecated and treated as Auto.
         assert_eq!(config.tls, TlsMode::Auto);
         assert_eq!(config.enforcement, EnforcementMode::Enforce);
     }
@@ -3421,30 +3390,37 @@ mod tests {
     }
 
     #[test]
-    fn validate_tls_terminate_deprecated_warning() {
-        let data = serde_json::json!({
-            "network_policies": {
-                "test": {
-                    "endpoints": [{
-                        "host": "api.example.com",
-                        "port": 443,
-                        "tls": "terminate",
-                        "protocol": "rest",
-                        "access": "full"
-                    }],
-                    "binaries": []
+    fn validate_rejects_unknown_endpoint_modes_without_warning() {
+        for (field, value) in [
+            ("tls", "terminate"),
+            ("tls", "passthrough"),
+            ("enforcement", "enforcee"),
+            ("access", "read_only"),
+        ] {
+            let mut endpoint = serde_json::json!({
+                "host": "api.example.com",
+                "port": 443,
+                "protocol": "rest",
+                "access": "full"
+            });
+            endpoint[field] = value.into();
+            let data = serde_json::json!({
+                "network_policies": {
+                    "test": { "endpoints": [endpoint], "binaries": [] }
                 }
-            }
-        });
-        let (errors, warnings) = validate_l7_policies(&data);
-        assert!(
-            errors.is_empty(),
-            "deprecated tls should not error: {errors:?}"
-        );
-        assert!(
-            warnings.iter().any(|w| w.contains("deprecated")),
-            "should warn about deprecated tls: {warnings:?}"
-        );
+            });
+
+            let (errors, warnings) = validate_l7_policies(&data);
+            assert!(
+                errors.iter().any(|e| e.contains("test.endpoints[0]")
+                    && e.contains(&format!("unknown {field} value '{value}'"))),
+                "{field}: {value} should be rejected: {errors:?}"
+            );
+            assert!(
+                !warnings.iter().any(|w| w.contains("deprecated")),
+                "{field}: {value} should not warn: {warnings:?}"
+            );
+        }
     }
 
     #[test]