Type in-container paths as POSIX on every host

sandbox/Cargo.toml

@@ -74,6 +74,7 @@ zenwave = { workspace = true, features = ["default"] }
 url = { version = "2.5.8", features = ["serde"] }
 procfs-core = { version = "0.18.0", default-features = false }
 fd-lock = "4"
+typed-path = "0.12.3"
 
 [features]
 default = []

sandbox/src/container.rs

@@ -8,6 +8,87 @@ use std::path::{Component, Path, PathBuf};
 
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
+use typed_path::{Utf8UnixPath, Utf8UnixPathBuf};
+
+/// An absolute POSIX path inside a container.
+///
+/// Container paths are joined with `/` whatever the host platform is, so a
+/// host [`Path`] can never stand in for one.
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)]
+#[serde(try_from = "String", into = "String")]
+#[schemars(with = "String")]
+pub struct ContainerPath(Utf8UnixPathBuf);
+
+/// A string that is not an absolute POSIX path.
+#[derive(Debug, thiserror::Error)]
+#[error("container path must be absolute: {0}")]
+pub struct RelativeContainerPath(pub String);
+
+impl ContainerPath {
+    /// The container's filesystem root, `/`.
+    #[must_use]
+    pub fn root() -> Self {
+        Self(Utf8UnixPathBuf::from("/"))
+    }
+
+    /// Parses an absolute POSIX path.
+    ///
+    /// # Errors
+    /// Returns [`RelativeContainerPath`] when `path` does not start at `/`.
+    pub fn new(path: impl Into<String>) -> Result<Self, RelativeContainerPath> {
+        let path = Utf8UnixPathBuf::from(path.into());
+        if path.is_absolute() {
+            Ok(Self(path))
+        } else {
+            Err(RelativeContainerPath(path.into_string()))
+        }
+    }
+
+    /// Appends `path` with POSIX separators; an absolute `path` replaces this one,
+    /// so the result is always absolute.
+    #[must_use]
+    pub fn join(&self, path: impl AsRef<str>) -> Self {
+        Self(self.0.join(path.as_ref()))
+    }
+
+    /// The containing directory, or `None` at the root.
+    #[must_use]
+    pub fn parent(&self) -> Option<Self> {
+        self.0.parent().map(|parent| Self(parent.to_path_buf()))
+    }
+
+    /// The path as POSIX text.
+    #[must_use]
+    pub fn as_str(&self) -> &str {
+        self.0.as_str()
+    }
+
+    /// The path as a POSIX path.
+    #[must_use]
+    pub fn as_unix_path(&self) -> &Utf8UnixPath {
+        &self.0
+    }
+}
+
+impl std::fmt::Display for ContainerPath {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(self.as_str())
+    }
+}
+
+impl TryFrom<String> for ContainerPath {
+    type Error = RelativeContainerPath;
+
+    fn try_from(path: String) -> Result<Self, Self::Error> {
+        Self::new(path)
+    }
+}
+
+impl From<ContainerPath> for String {
+    fn from(path: ContainerPath) -> Self {
+        path.0.into_string()
+    }
+}
 
 /// Supported Docker-compatible runtime kinds.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)]
@@ -74,7 +155,7 @@ pub struct MountSpec {
     /// Path on the host to expose.
     pub host_path: PathBuf,
     /// Path inside the container where it appears.
-    pub container_path: PathBuf,
+    pub container_path: ContainerPath,
     /// Whether the container may write to it.
     pub access: MountAccess,
 }
@@ -85,7 +166,7 @@ pub struct MountRoot {
     /// Base directory on the host.
     pub host_root: PathBuf,
     /// Directory the host root maps to inside the container.
-    pub container_root: PathBuf,
+    pub container_root: ContainerPath,
 }
 
 /// Mount-root composition error.
@@ -103,15 +184,19 @@ pub enum MountRootError {
     #[error("relative path must not contain parent traversal: {0}")]
     /// Relative path contained parent traversal.
     ParentTraversal(PathBuf),
+    /// A component is not UTF-8, so it has no container path spelling.
+    #[error("relative path must be valid UTF-8: {0}")]
+    /// Relative path contained a non-UTF-8 component.
+    NonUtf8(PathBuf),
 }
 
 impl MountRoot {
     /// Pairs a host directory with the container path it maps to.
     #[must_use]
-    pub fn new(host_root: impl Into<PathBuf>, container_root: impl Into<PathBuf>) -> Self {
+    pub fn new(host_root: impl Into<PathBuf>, container_root: ContainerPath) -> Self {
         Self {
             host_root: host_root.into(),
-            container_root: container_root.into(),
+            container_root,
         }
     }
 
@@ -120,17 +205,16 @@ impl MountRoot {
     /// # Errors
     ///
     /// Returns [`MountRootError`] if `relative` is empty, absolute, or contains
-    /// a `..` component — any of which would place the mount outside the root.
+    /// a `..` component — any of which would place the mount outside the root —
+    /// or if a component is not UTF-8.
     pub fn child(
         &self,
         relative: impl AsRef<Path>,
         access: MountAccess,
     ) -> Result<MountSpec, MountRootError> {
         let relative = relative.as_ref();
-        Self::validate_relative_path(relative)?;
-
+        let container_path = Self::container_child(&self.container_root, relative)?;
         let host_path = self.host_root.join(relative);
-        let container_path = self.container_root.join(relative);
         Ok(match access {
             MountAccess::ReadOnly => MountSpec::read_only(host_path, container_path),
             MountAccess::ReadWrite => MountSpec::read_write(host_path, container_path),
@@ -158,40 +242,54 @@ impl MountRoot {
         self.child(relative, MountAccess::ReadWrite)
     }
 
-    fn validate_relative_path(path: &Path) -> Result<(), MountRootError> {
-        if path.as_os_str().is_empty() {
-            return Err(MountRootError::EmptyRelativePath);
-        }
-        if path.is_absolute() {
-            return Err(MountRootError::AbsoluteRelativePath(path.to_path_buf()));
+    /// Joins the host-relative `path` onto `root` one component at a time, so
+    /// the container side uses POSIX separators whatever the host spells.
+    fn container_child(root: &ContainerPath, path: &Path) -> Result<ContainerPath, MountRootError> {
+        let mut container_path = root.clone();
+        let mut named = false;
+        for component in path.components() {
+            match component {
+                Component::Normal(name) => {
+                    let name = name
+                        .to_str()
+                        .ok_or_else(|| MountRootError::NonUtf8(path.to_path_buf()))?;
+                    container_path = container_path.join(name);
+                    named = true;
+                }
+                Component::CurDir => {}
+                Component::ParentDir => {
+                    return Err(MountRootError::ParentTraversal(path.to_path_buf()));
+                }
+                Component::RootDir | Component::Prefix(_) => {
+                    return Err(MountRootError::AbsoluteRelativePath(path.to_path_buf()));
+                }
+            }
         }
-        if path
-            .components()
-            .any(|component| matches!(component, Component::ParentDir))
-        {
-            return Err(MountRootError::ParentTraversal(path.to_path_buf()));
+        if named {
+            Ok(container_path)
+        } else {
+            Err(MountRootError::EmptyRelativePath)
         }
-        Ok(())
     }
 }
 
 impl MountSpec {
     /// Mounts a host path into the container without write access.
     #[must_use]
-    pub fn read_only(host_path: impl Into<PathBuf>, container_path: impl Into<PathBuf>) -> Self {
+    pub fn read_only(host_path: impl Into<PathBuf>, container_path: ContainerPath) -> Self {
         Self {
             host_path: host_path.into(),
-            container_path: container_path.into(),
+            container_path,
             access: MountAccess::ReadOnly,
         }
     }
 
     /// Mounts a host path into the container with write access.
     #[must_use]
-    pub fn read_write(host_path: impl Into<PathBuf>, container_path: impl Into<PathBuf>) -> Self {
+    pub fn read_write(host_path: impl Into<PathBuf>, container_path: ContainerPath) -> Self {
         Self {
             host_path: host_path.into(),
-            container_path: container_path.into(),
+            container_path,
             access: MountAccess::ReadWrite,
         }
     }
@@ -279,30 +377,77 @@ impl ContainerLaunchSpec {
 
 #[cfg(test)]
 mod tests {
-    use super::{MountRoot, MountRootError};
+    use super::{ContainerPath, MountRoot, MountRootError, MountSpec};
+
+    fn container_path(path: &str) -> ContainerPath {
+        ContainerPath::new(path).expect("test container paths are absolute")
+    }
 
     #[test]
     fn mount_root_maps_relative_path() {
-        let root = MountRoot::new("/host/.may", "/root/.may");
+        let root = MountRoot::new("/host/.may", container_path("/root/.may"));
         let mount = root
             .read_only_child("skills")
             .expect("relative child mount should be valid");
         assert_eq!(
             mount.host_path,
             std::path::PathBuf::from("/host/.may/skills")
         );
-        assert_eq!(
-            mount.container_path,
-            std::path::PathBuf::from("/root/.may/skills")
-        );
+        assert_eq!(mount.container_path.as_str(), "/root/.may/skills");
+    }
+
+    #[test]
+    fn mount_root_joins_nested_children_with_posix_separators() {
+        let root = MountRoot::new("/host/.may", container_path("/root/.may"));
+        let relative = std::path::Path::new("skills").join("python");
+        let mount = root
+            .read_write_child(relative)
+            .expect("nested child mount should be valid");
+        assert_eq!(mount.container_path.as_str(), "/root/.may/skills/python");
     }
 
     #[test]
     fn mount_root_rejects_parent_traversal() {
-        let root = MountRoot::new("/host/.may", "/root/.may");
+        let root = MountRoot::new("/host/.may", container_path("/root/.may"));
         let error = root
             .read_write_child("../secrets")
             .expect_err("parent traversal must fail");
         assert!(matches!(error, MountRootError::ParentTraversal(_)));
     }
+
+    #[test]
+    fn mount_root_rejects_empty_and_rooted_children() {
+        let root = MountRoot::new("/host/.may", container_path("/root/.may"));
+        assert!(matches!(
+            root.read_only_child("."),
+            Err(MountRootError::EmptyRelativePath)
+        ));
+        assert!(matches!(
+            root.read_only_child("/etc"),
+            Err(MountRootError::AbsoluteRelativePath(_))
+        ));
+    }
+
+    #[test]
+    fn container_paths_are_absolute_posix_paths() {
+        assert!(ContainerPath::new("opt/aither").is_err());
+        let path = ContainerPath::root().join("opt").join("aither/tools");
+        assert_eq!(path.as_str(), "/opt/aither/tools");
+        assert_eq!(path.parent().unwrap().as_str(), "/opt/aither");
+        assert_eq!(path.join("/etc").as_str(), "/etc");
+    }
+
+    #[test]
+    fn mount_spec_serializes_container_path_as_text_and_rejects_relative() {
+        let mount = MountSpec::read_only("/host/tools", container_path("/opt/aither/tools/key"));
+        let json = serde_json::to_value(&mount).unwrap();
+        assert_eq!(json["container_path"], "/opt/aither/tools/key");
+        assert_eq!(serde_json::from_value::<MountSpec>(json).unwrap(), mount);
+        let relative = serde_json::json!({
+            "host_path": "/host/tools",
+            "container_path": "opt/aither/tools",
+            "access": "read_only",
+        });
+        assert!(serde_json::from_value::<MountSpec>(relative).is_err());
+    }
 }

sandbox/src/lib.rs

@@ -85,8 +85,8 @@ pub use command::{
     register_ipc_gateway_command, register_tool_command, register_tool_direct, schema_to_help,
 };
 pub use container::{
-    ContainerImageSpec, ContainerLaunchSpec, ContainerRuntimeKind, MountAccess, MountRoot,
-    MountRootError, MountSpec, RuntimePreference,
+    ContainerImageSpec, ContainerLaunchSpec, ContainerPath, ContainerRuntimeKind, MountAccess,
+    MountRoot, MountRootError, MountSpec, RelativeContainerPath, RuntimePreference,
 };
 /// Re-export of heel's rolling network audit log for terminal-tool configuration.
 pub use heel::NetworkAuditLog;