fix(policy): harden landlock.compatibility validation

.gitignore

@@ -227,6 +227,9 @@ rfc.md
 # Markdown/mermaid lint tooling deps
 scripts/lint-mermaid/node_modules/
 
+# JS/TS dependencies
+node_modules/
+
 # Nix
 /result
 /result-*

crates/openshell-core/src/policy.rs

@@ -92,6 +92,19 @@ pub enum LandlockCompatibility {
     HardRequirement,
 }
 
+/// Accepted `landlock.compatibility` values in their proto string form.
+///
+/// Single source of truth shared by YAML parsing, proto→runtime conversion,
+/// and gateway policy validation so the accepted set cannot drift.
+pub const LANDLOCK_COMPATIBILITY_VALUES: [&str; 2] = ["best_effort", "hard_requirement"];
+
+/// Returns `true` if `value` is an accepted `landlock.compatibility` string.
+///
+/// The empty string is accepted and defaults to `best_effort`.
+pub fn is_valid_landlock_compatibility(value: &str) -> bool {
+    value.is_empty() || LANDLOCK_COMPATIBILITY_VALUES.contains(&value)
+}
+
 // ============================================================================
 // Proto to Rust type conversions
 // ============================================================================
@@ -114,7 +127,11 @@ impl TryFrom<ProtoSandboxPolicy> for SandboxPolicy {
                 .map(FilesystemPolicy::from)
                 .unwrap_or_default(),
             network,
-            landlock: proto.landlock.map(LandlockPolicy::from).unwrap_or_default(),
+            landlock: proto
+                .landlock
+                .map(LandlockPolicy::try_from)
+                .transpose()?
+                .unwrap_or_default(),
             process: proto.process.map(ProcessPolicy::from).unwrap_or_default(),
         })
     }
@@ -138,14 +155,20 @@ impl From<ProtoFilesystemPolicy> for FilesystemPolicy {
     }
 }
 
-impl From<ProtoLandlockPolicy> for LandlockPolicy {
-    fn from(proto: ProtoLandlockPolicy) -> Self {
-        let compatibility = if proto.compatibility == "hard_requirement" {
-            LandlockCompatibility::HardRequirement
-        } else {
-            LandlockCompatibility::BestEffort
+impl TryFrom<ProtoLandlockPolicy> for LandlockPolicy {
+    type Error = miette::Error;
+
+    fn try_from(proto: ProtoLandlockPolicy) -> Result<Self, Self::Error> {
+        let compatibility = match proto.compatibility.as_str() {
+            "best_effort" | "" => LandlockCompatibility::BestEffort,
+            "hard_requirement" => LandlockCompatibility::HardRequirement,
+            otherwise => miette::bail!(
+                "invalid landlock.compatibility {:?}; accepted: {}",
+                otherwise,
+                LANDLOCK_COMPATIBILITY_VALUES.join(", ")
+            ),
         };
-        Self { compatibility }
+        Ok(Self { compatibility })
     }
 }
 
@@ -165,3 +188,49 @@ impl From<ProtoProcessPolicy> for ProcessPolicy {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn try_from_maps_known_compatibility_values() {
+        for (input, expected) in [
+            ("", LandlockCompatibility::BestEffort),
+            ("best_effort", LandlockCompatibility::BestEffort),
+            ("hard_requirement", LandlockCompatibility::HardRequirement),
+        ] {
+            let proto = ProtoLandlockPolicy {
+                compatibility: input.into(),
+            };
+            let policy = LandlockPolicy::try_from(proto).expect("should convert");
+            assert_eq!(
+                std::mem::discriminant(&policy.compatibility),
+                std::mem::discriminant(&expected),
+                "input {input:?} mapped to unexpected variant",
+            );
+        }
+    }
+
+    #[test]
+    fn try_from_rejects_invalid_compatibility() {
+        let proto = ProtoLandlockPolicy {
+            compatibility: "hard-requirement".into(),
+        };
+        let err = LandlockPolicy::try_from(proto).expect_err("should reject");
+        let msg = format!("{err:?}");
+        assert!(
+            msg.contains("best_effort") && msg.contains("hard_requirement"),
+            "error should list accepted values, got: {msg}",
+        );
+    }
+
+    #[test]
+    fn is_valid_landlock_compatibility_accepts_empty_and_known() {
+        assert!(is_valid_landlock_compatibility(""));
+        assert!(is_valid_landlock_compatibility("best_effort"));
+        assert!(is_valid_landlock_compatibility("hard_requirement"));
+        assert!(!is_valid_landlock_compatibility("nope"));
+        assert!(!is_valid_landlock_compatibility("BestEffort"));
+    }
+}

crates/openshell-policy/src/lib.rs

@@ -70,11 +70,19 @@ struct FilesystemDef {
     read_write: Vec<String>,
 }
 
+#[derive(Debug, Default, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+enum LandlockCompatibilityDef {
+    #[default]
+    BestEffort,
+    HardRequirement,
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 struct LandlockDef {
-    #[serde(default, skip_serializing_if = "String::is_empty")]
-    compatibility: String,
+    #[serde(default)]
+    compatibility: LandlockCompatibilityDef,
 }
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -781,7 +789,10 @@ fn to_proto(raw: PolicyFile) -> Result<SandboxPolicy> {
             read_write: fs.read_write,
         }),
         landlock: raw.landlock.map(|ll| LandlockPolicy {
-            compatibility: ll.compatibility,
+            compatibility: match ll.compatibility {
+                LandlockCompatibilityDef::BestEffort => "best_effort".to_string(),
+                LandlockCompatibilityDef::HardRequirement => "hard_requirement".to_string(),
+            },
         }),
         process: raw.process.map(|p| ProcessPolicy {
             run_as_user: p.run_as_user,
@@ -804,7 +815,20 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile {
     });
 
     let landlock = policy.landlock.as_ref().map(|ll| LandlockDef {
-        compatibility: ll.compatibility.clone(),
+        compatibility: {
+            // Persisted values are validated at create time
+            // (`validate_sandbox_policy`), so an unknown value here indicates a
+            // regression rather than untrusted input.
+            debug_assert!(
+                openshell_core::policy::is_valid_landlock_compatibility(&ll.compatibility),
+                "unvalidated landlock.compatibility reached from_proto: {:?}",
+                ll.compatibility,
+            );
+            match ll.compatibility.as_str() {
+                "hard_requirement" => LandlockCompatibilityDef::HardRequirement,
+                _ => LandlockCompatibilityDef::BestEffort,
+            }
+        },
     });
 
     let process = policy.process.as_ref().and_then(|p| {
@@ -1156,6 +1180,8 @@ pub enum PolicyViolation {
         policy_name: String,
         host: String,
     },
+    /// `landlock.compatibility` has an unrecognized value.
+    InvalidLandlockCompatibility { value: String },
 }
 
 impl fmt::Display for PolicyViolation {
@@ -1266,6 +1292,13 @@ impl fmt::Display for PolicyViolation {
                      '{policy_name}' tls: skip endpoint '{host}'"
                 )
             }
+            Self::InvalidLandlockCompatibility { value } => {
+                write!(
+                    f,
+                    "invalid landlock.compatibility '{value}'; accepted: {}",
+                    openshell_core::policy::LANDLOCK_COMPATIBILITY_VALUES.join(", ")
+                )
+            }
         }
     }
 }
@@ -1311,6 +1344,17 @@ pub fn validate_sandbox_policy(
         }
     }
 
+    // Check landlock compatibility mode is a recognized value. Direct gRPC/SDK
+    // clients bypass YAML serde validation, so reject invalid values here at the
+    // gateway create path rather than deferring rejection to sandbox startup.
+    if let Some(ref landlock) = policy.landlock
+        && !openshell_core::policy::is_valid_landlock_compatibility(&landlock.compatibility)
+    {
+        violations.push(PolicyViolation::InvalidLandlockCompatibility {
+            value: landlock.compatibility.clone(),
+        });
+    }
+
     // Check filesystem paths
     if let Some(ref fs) = policy.filesystem {
         let total_paths = fs.read_only.len() + fs.read_write.len();
@@ -1965,6 +2009,54 @@ network_policies:
         assert_eq!(violations.len(), 2);
     }
 
+    #[test]
+    fn parse_rejects_invalid_landlock_compatibility() {
+        let err = parse_sandbox_policy("version: 1\nlandlock:\n  compatibility: bogus\n")
+            .expect_err("should reject invalid YAML enum value");
+        let msg = format!("{err:?}");
+        assert!(
+            msg.contains("best_effort") && msg.contains("hard_requirement"),
+            "error should list accepted values, got: {msg}",
+        );
+    }
+
+    #[test]
+    fn parse_accepts_known_landlock_compatibility() {
+        for value in ["best_effort", "hard_requirement"] {
+            let yaml = format!("version: 1\nlandlock:\n  compatibility: {value}\n");
+            let policy = parse_sandbox_policy(&yaml).expect("should parse");
+            assert_eq!(
+                policy.landlock.as_ref().expect("landlock").compatibility,
+                value,
+            );
+        }
+    }
+
+    #[test]
+    fn validate_rejects_invalid_landlock_compatibility_proto() {
+        let mut policy = restrictive_default_policy();
+        policy.landlock = Some(LandlockPolicy {
+            compatibility: "nope".into(),
+        });
+        let violations = validate_sandbox_policy(&policy).unwrap_err();
+        assert!(
+            violations
+                .iter()
+                .any(|v| matches!(v, PolicyViolation::InvalidLandlockCompatibility { .. })),
+            "expected InvalidLandlockCompatibility, got: {violations:?}",
+        );
+    }
+
+    #[test]
+    fn validate_accepts_empty_landlock_compatibility() {
+        // Empty string is the proto default and maps to best_effort.
+        let mut policy = restrictive_default_policy();
+        policy.landlock = Some(LandlockPolicy {
+            compatibility: String::new(),
+        });
+        assert!(validate_sandbox_policy(&policy).is_ok());
+    }
+
     #[test]
     fn validate_rejects_invalid_middleware_control_fields() {
         let cases = [

crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs

@@ -144,6 +144,14 @@ fn prepare_with_path_open_mode(
     }
 
     if read_only.is_empty() && read_write.is_empty() {
+        if matches!(
+            policy.landlock.compatibility,
+            LandlockCompatibility::HardRequirement
+        ) {
+            miette::bail!(
+                "landlock.compatibility is hard_requirement but no filesystem paths are configured"
+            );
+        }
         return Ok(None);
     }
 
@@ -522,6 +530,69 @@ mod tests {
             panic!("hard_requirement should accept mixed directory and device paths: {err}");
         }
     }
+    #[test]
+    fn prepare_hard_requirement_no_paths_aborts() {
+        // Zero configured paths under hard_requirement must fail startup rather
+        // than silently running without filesystem restrictions.
+        let policy = hard_requirement_policy(vec![], vec![]);
+        let err = match prepare(&policy, None) {
+            Err(err) => err,
+            Ok(_) => panic!("should abort with no paths"),
+        };
+        let msg = err.to_string();
+        assert!(
+            msg.contains("hard_requirement") && msg.contains("no filesystem paths"),
+            "error should explain the empty hard_requirement policy: {msg}"
+        );
+    }
+
+    #[test]
+    fn prepare_best_effort_no_paths_is_noop() {
+        let policy = SandboxPolicy {
+            version: 1,
+            filesystem: FilesystemPolicy {
+                read_only: vec![],
+                read_write: vec![],
+                include_workdir: false,
+            },
+            network: NetworkPolicy::default(),
+            landlock: LandlockPolicy {
+                compatibility: LandlockCompatibility::BestEffort,
+            },
+            process: ProcessPolicy::default(),
+        };
+        let prepared = prepare(&policy, None).expect("best_effort no-op should succeed");
+        assert!(prepared.is_none(), "no paths should produce no ruleset");
+    }
+
+    #[test]
+    fn prepare_include_workdir_counts_as_configured_path() {
+        // With no explicit paths but include_workdir set, the workdir must be
+        // treated as a configured path — so the zero-path abort must NOT fire.
+        let policy = SandboxPolicy {
+            version: 1,
+            filesystem: FilesystemPolicy {
+                read_only: vec![],
+                read_write: vec![],
+                include_workdir: true,
+            },
+            network: NetworkPolicy::default(),
+            landlock: LandlockPolicy {
+                compatibility: LandlockCompatibility::HardRequirement,
+            },
+            process: ProcessPolicy::default(),
+        };
+        // Any error (e.g. Landlock unavailable on this host) is acceptable, but
+        // it must not be the "no filesystem paths" abort.
+        if let Err(err) = prepare(&policy, Some("/tmp")) {
+            let msg = err.to_string();
+            assert!(
+                !msg.contains("no filesystem paths"),
+                "workdir should count as a configured path: {msg}"
+            );
+        }
+    }
+
     fn tailored_access(path: &Path, requested_access: BitFlags<AccessFs>) -> BitFlags<AccessFs> {
         let path_fd = PathFd::new(path).unwrap();
         access_for_path_fd(&path_fd, requested_access, ABI::V2).unwrap()