From ea6f9b678d85e1328246269e28c4760f3d4b46af Mon Sep 17 00:00:00 2001 From: Lexo Liu Date: Thu, 15 Oct 2026 04:05:41 -0400 Subject: [PATCH] fix(sandbox): type in-container paths as POSIX on every host Container-side paths (the tool mount directory and its bin, mount destinations, the missing read-only mount error, the container mount check) were built with host PathBufs, so a Windows host produced /opt/aither/tools\\bin inside a Linux container. They are now a ContainerPath (typed-path's POSIX path, absolute only, always joined with /), so a host path cannot be used where a container path is expected. Prepared tools are typed by where they run, and the terminal passes one value per backend instead of five optional fields, which removes three runtime "missing runtime" errors. Closes #135 --- Cargo.lock | 7 + sandbox/Cargo.toml | 1 + sandbox/src/container.rs | 207 ++++++++++++++++++---- sandbox/src/lib.rs | 4 +- sandbox/src/terminal.rs | 296 +++++++++++++------------------- sandbox/src/tool_environment.rs | 112 ++++++------ sandbox/src/tool_runtime.rs | 130 +++++++++++--- sandbox/src/tool_transport.rs | 29 ++-- 8 files changed, 495 insertions(+), 291 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a6669720..8ab5387f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -541,6 +541,7 @@ dependencies = [ "tokio", "tracing", "tree-sitter", + "typed-path", "url", "uuid", "vte", @@ -8685,6 +8686,12 @@ dependencies = [ "rand 0.10.2", ] +[[package]] +name = "typed-path" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e28f89b80c87b8fb0cf04ab448d5dd0dd0ade2f8891bae878de66a75a28600e" + [[package]] name = "typenum" version = "1.20.1" diff --git a/sandbox/Cargo.toml b/sandbox/Cargo.toml index 0e5699f2..7039e889 100644 --- a/sandbox/Cargo.toml +++ b/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 = [] diff --git a/sandbox/src/container.rs b/sandbox/src/container.rs index b79291c0..efec9f30 100644 --- a/sandbox/src/container.rs +++ b/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) -> Result { + 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) -> Self { + Self(self.0.join(path.as_ref())) + } + + /// The containing directory, or `None` at the root. + #[must_use] + pub fn parent(&self) -> Option { + 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 for ContainerPath { + type Error = RelativeContainerPath; + + fn try_from(path: String) -> Result { + Self::new(path) + } +} + +impl From 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, container_root: impl Into) -> Self { + pub fn new(host_root: impl Into, 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, access: MountAccess, ) -> Result { 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 { + 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, container_path: impl Into) -> Self { + pub fn read_only(host_path: impl Into, 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, container_path: impl Into) -> Self { + pub fn read_write(host_path: impl Into, container_path: ContainerPath) -> Self { Self { host_path: host_path.into(), - container_path: container_path.into(), + container_path, access: MountAccess::ReadWrite, } } @@ -279,11 +377,15 @@ 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"); @@ -291,18 +393,61 @@ mod tests { 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::(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::(relative).is_err()); + } } diff --git a/sandbox/src/lib.rs b/sandbox/src/lib.rs index 81943895..f2517fb5 100644 --- a/sandbox/src/lib.rs +++ b/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; diff --git a/sandbox/src/terminal.rs b/sandbox/src/terminal.rs index 1c0b69f4..53e1c1b4 100644 --- a/sandbox/src/terminal.rs +++ b/sandbox/src/terminal.rs @@ -44,7 +44,7 @@ use shell_words::split as split_shell_words; use tracing::{debug, info, warn}; use crate::{ - PreparedTools, ToolDistribution, ToolRuntimeError, + ContainerPath, PreparedTools, ToolDistribution, ToolRuntimeError, builtin::builtin_router, command::ToolRegistry, job_registry::{JobRegistry, job_registry_channel}, @@ -1072,12 +1072,9 @@ where } /// Prepared tools of the local home backend, when this terminal runs on it. - fn local_home_tools(&self) -> Result, ToolRuntimeError> { + fn local_home_tools(&self) -> Result>, ToolRuntimeError> { match self.shell_sessions.resolve_local_backend() { - Ok(ShellBackend::Local) => self - .shell_sessions - .home_tools(ShellBackend::Local) - .map(Some), + Ok(ShellBackend::Local) => self.shell_sessions.local_tools().map(Some), _ => Ok(None), } } @@ -1285,24 +1282,21 @@ where "unsafe mode is only available in heel local runtime".into(), )); } - let container_runtime = if backend == ShellBackend::Container { - Some( - self.shell_sessions + let backend = match backend { + ShellBackend::Local => { + ExecutionBackend::Local(self.shell_sessions.local_tools()?.clone()) + } + ShellBackend::Container => ExecutionBackend::Container { + runtime: self + .shell_sessions .container_runtime() .cloned() .ok_or(ToolRuntimeError::MissingContainerRuntime)?, - ) - } else { - None + tools: self.shell_sessions.container_tools()?.clone(), + }, + ShellBackend::Ssh => unreachable!("the home backend is never SSH"), }; - Ok(ExecutionTarget { - backend, - mode, - ssh_target: None, - ssh_runtime: None, - container_runtime, - prepared_tools: self.shell_sessions.home_tools(backend)?.clone(), - }) + Ok(ExecutionTarget { mode, backend }) } fn resolve_ssh_target( @@ -1320,12 +1314,12 @@ where .map_err(TerminalError::InvalidTarget)?; let (runtime, tools) = self.shell_sessions.ssh_tools(&server)?; Ok(ExecutionTarget { - backend: ShellBackend::Ssh, mode: TerminalMode::Sandboxed, - ssh_target: Some(server.target), - ssh_runtime: Some(runtime.clone()), - container_runtime: None, - prepared_tools: tools.clone(), + backend: ExecutionBackend::Ssh { + target: server.target, + runtime: runtime.clone(), + tools: tools.clone(), + }, }) } @@ -1379,19 +1373,12 @@ where raw, timeout, } = request; - let ExecutionTarget { - backend, - mode, - ssh_target, - ssh_runtime, - container_runtime, - prepared_tools, - } = target; + let ExecutionTarget { mode, backend } = target; let (result_tx, result_rx) = async_channel::bounded(1); let (startup_tx, startup_rx) = async_channel::bounded(1); let (stdin_blocked_tx, stdin_blocked_rx) = async_channel::bounded(1); let background_mode = Arc::new(AtomicBool::new(timeout == 0)); - let stdin_blocked_notice = if supports_stdin_blocked_notice(backend) { + let stdin_blocked_notice = if supports_stdin_blocked_notice(backend.kind()) { Some(stdin_blocked_tx) } else { None @@ -1419,10 +1406,6 @@ where script, mode, backend, - ssh_target, - ssh_runtime, - container_runtime, - prepared_tools, stdin_blocked_notice, background_startup, expect, @@ -1475,32 +1458,18 @@ impl Tool let raw = arguments.raw; let timeout = arguments.timeout; - let ExecutionTarget { - backend, - mode, - ssh_target, - ssh_runtime, - container_runtime, - prepared_tools, - } = self.resolve_execution_target(&arguments)?; - if backend == ShellBackend::Local { - prepared_tools + let target = self.resolve_execution_target(&arguments)?; + if let ExecutionBackend::Local(tools) = &target.backend { + tools .ensure_readonly( std::iter::once(&self.working_dir).chain(self.writable_paths.iter()), ) .await?; } - self.ensure_permission_allowed(mode, &script).await?; + self.ensure_permission_allowed(target.mode, &script).await?; let store_dir = self.output_store.dir().to_path_buf(); let spawned = self.spawn_terminal_execution(TerminalSpawnRequest { - target: ExecutionTarget { - backend, - mode, - ssh_target, - ssh_runtime, - container_runtime, - prepared_tools, - }, + target, task_id: task_id.clone(), execution_id, script, @@ -1697,11 +1666,7 @@ struct StandaloneExecution { background_mode: Arc, script: String, mode: TerminalMode, - backend: ShellBackend, - ssh_target: Option, - ssh_runtime: Option, - container_runtime: Option, - prepared_tools: PreparedTools, + backend: ExecutionBackend, stdin_blocked_notice: Option>, background_startup: BackgroundStartup, network_audit: Option, @@ -1725,11 +1690,7 @@ struct StandaloneStart<'a, P, E> { execution_id: &'a str, script: &'a str, mode: TerminalMode, - backend: ShellBackend, - ssh_target: Option<&'a str>, - ssh_runtime: Option, - container_runtime: Option<&'a ContainerShellRuntime>, - prepared_tools: &'a PreparedTools, + backend: &'a ExecutionBackend, stdin_blocked_notice: Option>, background_startup: &'a BackgroundStartup, network_audit: Option, @@ -1737,7 +1698,7 @@ struct StandaloneStart<'a, P, E> { } struct LocalBackgroundExecution<'a, E> { - prepared_tools: &'a PreparedTools, + prepared_tools: &'a PreparedTools, working_dir: &'a Path, writable_paths: &'a [PathBuf], readable_paths: &'a [PathBuf], @@ -1760,8 +1721,8 @@ struct ContainerBackgroundExecution<'a, E> { execution_id: &'a str, script: &'a str, mode: TerminalMode, - container_runtime: Option<&'a ContainerShellRuntime>, - prepared_tools: &'a PreparedTools, + container_runtime: &'a ContainerShellRuntime, + prepared_tools: &'a PreparedTools, ipc_commands: &'a [String], job_registry: &'a JobRegistry, stdin_blocked_notice: Option>, @@ -1770,13 +1731,13 @@ struct ContainerBackgroundExecution<'a, E> { } struct SshBackgroundExecution<'a> { - prepared_tools: &'a PreparedTools, + prepared_tools: &'a PreparedTools, task_id: &'a str, execution_id: &'a str, script: &'a str, mode: TerminalMode, - ssh_target: Option<&'a str>, - ssh_runtime: Option, + ssh_target: &'a str, + ssh_runtime: &'a SshRuntimeProfile, job_registry: &'a JobRegistry, background_startup: &'a BackgroundStartup, } @@ -1793,13 +1754,36 @@ struct CompletedProcessOutput<'a> { raw: bool, } +/// Where a command runs, with the tools prepared for exactly that environment. +enum ExecutionBackend { + /// The local sandbox, whose tools live at a host path. + Local(PreparedTools), + /// A container, whose tools are mounted at a container path. + Container { + runtime: ContainerShellRuntime, + tools: PreparedTools, + }, + /// A remote host over SSH. + Ssh { + target: String, + runtime: SshRuntimeProfile, + tools: PreparedTools, + }, +} + +impl ExecutionBackend { + const fn kind(&self) -> ShellBackend { + match self { + Self::Local(_) => ShellBackend::Local, + Self::Container { .. } => ShellBackend::Container, + Self::Ssh { .. } => ShellBackend::Ssh, + } + } +} + struct ExecutionTarget { - backend: ShellBackend, mode: TerminalMode, - ssh_target: Option, - ssh_runtime: Option, - container_runtime: Option, - prepared_tools: PreparedTools, + backend: ExecutionBackend, } struct TerminalSpawnRequest { @@ -1834,11 +1818,7 @@ struct SpawnedTerminalTask { background_mode: Arc, script: String, mode: TerminalMode, - backend: ShellBackend, - ssh_target: Option, - ssh_runtime: Option, - container_runtime: Option, - prepared_tools: PreparedTools, + backend: ExecutionBackend, stdin_blocked_notice: Option>, background_startup: BackgroundStartup, expect: OutputFormat, @@ -1873,10 +1853,6 @@ where script, mode, backend, - ssh_target, - ssh_runtime, - container_runtime, - prepared_tools, stdin_blocked_notice, background_startup, expect, @@ -1905,10 +1881,6 @@ where script: script.clone(), mode, backend, - ssh_target, - ssh_runtime, - container_runtime, - prepared_tools, stdin_blocked_notice, background_startup, network_audit, @@ -1960,10 +1932,6 @@ where script, mode, backend, - ssh_target, - ssh_runtime, - container_runtime, - prepared_tools, stdin_blocked_notice, background_startup, network_audit, @@ -1993,11 +1961,7 @@ where execution_id: &execution_id, script: &script, mode, - backend, - ssh_target: ssh_target.as_deref(), - ssh_runtime, - container_runtime: container_runtime.as_ref(), - prepared_tools: &prepared_tools, + backend: &backend, stdin_blocked_notice, background_startup: &background_startup, network_audit, @@ -2048,41 +2012,48 @@ where P: PermissionHandler + 'static, E: Executor + Clone + 'static, { - if matches!(request.backend, ShellBackend::Container) { - let ipc_commands = request.registry.registered_tool_names(); - return execute_container_background(ContainerBackgroundExecution { - executor: request.executor, - registry: request.registry, - task_id: request.task_id, - execution_id: request.execution_id, - script: request.script, - mode: request.mode, - container_runtime: request.container_runtime, - prepared_tools: request.prepared_tools, - ipc_commands: &ipc_commands, - job_registry: request.job_registry, - stdin_blocked_notice: request.stdin_blocked_notice, - background_startup: request.background_startup, - priority: request.priority, - }) - .await; - } - if matches!(request.backend, ShellBackend::Ssh) { - return execute_ssh_background(SshBackgroundExecution { - prepared_tools: request.prepared_tools, - task_id: request.task_id, - execution_id: request.execution_id, - script: request.script, - mode: request.mode, - ssh_target: request.ssh_target, - ssh_runtime: request.ssh_runtime, - job_registry: request.job_registry, - background_startup: request.background_startup, - }) - .await; - } + let prepared_tools = match request.backend { + ExecutionBackend::Local(tools) => tools, + ExecutionBackend::Container { runtime, tools } => { + let ipc_commands = request.registry.registered_tool_names(); + return execute_container_background(ContainerBackgroundExecution { + executor: request.executor, + registry: request.registry, + task_id: request.task_id, + execution_id: request.execution_id, + script: request.script, + mode: request.mode, + container_runtime: runtime, + prepared_tools: tools, + ipc_commands: &ipc_commands, + job_registry: request.job_registry, + stdin_blocked_notice: request.stdin_blocked_notice, + background_startup: request.background_startup, + priority: request.priority, + }) + .await; + } + ExecutionBackend::Ssh { + target, + runtime, + tools, + } => { + return execute_ssh_background(SshBackgroundExecution { + prepared_tools: tools, + task_id: request.task_id, + execution_id: request.execution_id, + script: request.script, + mode: request.mode, + ssh_target: target, + ssh_runtime: runtime, + job_registry: request.job_registry, + background_startup: request.background_startup, + }) + .await; + } + }; let local = LocalBackgroundExecution { - prepared_tools: request.prepared_tools, + prepared_tools, working_dir: request.working_dir, writable_paths: request.writable_paths, readable_paths: request.readable_paths, @@ -2266,7 +2237,7 @@ async fn save_completed_stderr( fn local_path_grants<'a>( writable: &'a [PathBuf], readable: &'a [PathBuf], - tools: &'a PreparedTools, + tools: &'a PreparedTools, ) -> impl Iterator + 'a { writable .iter() @@ -2515,9 +2486,7 @@ async fn wait_for_local_process_exit( async fn execute_container_background( context: ContainerBackgroundExecution<'_, E>, ) -> Result<(u32, std::process::Output), TerminalError> { - let container_runtime = context.container_runtime.ok_or_else(|| { - TerminalError::Execution("missing container runtime for container backend".into()) - })?; + let container_runtime = context.container_runtime; let exec = container_runtime.exec(); // Use lower 32 bits of a UUID as a synthetic PID for job tracking. @@ -2626,22 +2595,14 @@ struct SshToolExecutionTemplate<'a> { async fn execute_ssh_background( context: SshBackgroundExecution<'_>, ) -> Result<(u32, std::process::Output), TerminalError> { - let target = context - .ssh_target - .ok_or_else(|| TerminalError::Execution("missing ssh target".to_string()))?; - let runtime = context - .ssh_runtime - .ok_or_else(|| TerminalError::Execution("missing ssh runtime profile".to_string()))?; - - let remote_cmd = match (runtime, context.mode) { + let target = context.ssh_target; + let remote_cmd = match (context.ssh_runtime, context.mode) { (SshRuntimeProfile::Heel { binary }, TerminalMode::Sandboxed) => { let script = context.prepared_tools.wrap_script(context.script)?; let (program, args) = shell_launch(&script); - let directory = context.prepared_tools.bin_dir().to_str().ok_or_else(|| { - ToolRuntimeError::InvalidCache(context.prepared_tools.bin_dir().clone()) - })?; + let directory = context.prepared_tools.script_bin_dir()?; SshToolExecutionTemplate { - binary: shell_words::quote(&binary), + binary: shell_words::quote(binary), tools_dir: shell_words::quote(directory), program: shell_words::quote(&program), arguments: shell_words::join(&args), @@ -2726,7 +2687,7 @@ fn shell_escape(value: &str) -> String { async fn prepare_session_runtime_environment( working_dir: &Path, - tools: &PreparedTools, + tools: &PreparedTools, ) -> Result, TerminalError> { let session_tmp = working_dir.join("tmp"); let session_cache = working_dir.join(".cache"); @@ -3638,7 +3599,10 @@ mod tests { ssh: false, }) .with_container_runtime(test_container_runtime("aither-tools-a")) - .with_test_container_tools(tools, "aither-tools-a".to_string()); + .with_test_container_tools( + crate::tool_runtime::tests::container_tools(&crate::tool_runtime::tests::bundle()), + "aither-tools-a".to_string(), + ); let dir = tempfile::tempdir().unwrap(); let adopt = |sessions: ShellSessionRegistry| { @@ -4276,15 +4240,7 @@ mod tests { background_mode: Arc::new(AtomicBool::new(false)), script: "/bin/pwd".to_string(), mode: TerminalMode::Sandboxed, - backend: ShellBackend::Local, - ssh_target: None, - ssh_runtime: None, - container_runtime: None, - prepared_tools: tool - .shell_sessions - .home_tools(ShellBackend::Local) - .unwrap() - .clone(), + backend: ExecutionBackend::Local(tool.shell_sessions.local_tools().unwrap().clone()), stdin_blocked_notice: None, background_startup: BackgroundStartup::new(startup_tx), network_audit: None, @@ -5054,21 +5010,9 @@ mod tests { // The container backend needs its own prepared tools: the registry // entry names the container and its `/opt/aither/tools` bin dir, which // the wrapped script puts on `PATH`. Nothing in this test invokes a - // prepared command, so preparing the catalog on the host and pointing - // the bin dir at the container path is enough. - let tools_cache = tempfile::tempdir().expect("tools cache should be created"); - let bundle = crate::tool_runtime::tests::bundle(); - let prepared = bundle - .prepare_local(tools_cache.path()) - .await - .expect("test tools should prepare"); - let container_tools = crate::tool_runtime::PreparedTools { - bin_dir: bundle - .container_directory() - .expect("the bundle has a container directory") - .join("bin"), - catalog: prepared.catalog().clone(), - }; + // prepared command, so the bundle's container view is enough. + let container_tools = + crate::tool_runtime::tests::container_tools(&crate::tool_runtime::tests::bundle()); let shell_sessions = ShellSessionRegistry::new(ShellRuntimeAvailability { local: false, container: true, diff --git a/sandbox/src/tool_environment.rs b/sandbox/src/tool_environment.rs index 3a693b7b..276d19ff 100644 --- a/sandbox/src/tool_environment.rs +++ b/sandbox/src/tool_environment.rs @@ -1,7 +1,8 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use futures::{StreamExt, TryStreamExt, stream}; +use crate::ContainerPath; use crate::shell_session::{ ShellBackend, ShellSessionRegistry, SshRuntimeProfile, SshServer, SshSessionAuthorizer, bootstrap_ssh_runtime, @@ -12,18 +13,37 @@ use crate::tool_runtime::{ }; use crate::tool_transport::{ContainerToolTransport, SshToolTransport, probe_tool_environment}; +/// The home backend's tools, typed by where that backend sees them. #[derive(Debug, Clone)] -struct PreparedHome { - backend: ShellBackend, - container_id: Option, - tools: PreparedTools, +enum PreparedHome { + Local(PreparedTools), + Container { + container_id: String, + tools: PreparedTools, + }, +} + +impl PreparedHome { + const fn backend(&self) -> ShellBackend { + match self { + Self::Local(_) => ShellBackend::Local, + Self::Container { .. } => ShellBackend::Container, + } + } + + const fn catalog(&self) -> &ToolCatalog { + match self { + Self::Local(tools) => tools.catalog(), + Self::Container { tools, .. } => tools.catalog(), + } + } } #[derive(Debug, Clone)] struct PreparedSsh { server: SshServer, runtime: SshRuntimeProfile, - tools: PreparedTools, + tools: PreparedTools, } #[derive(Debug, Clone)] @@ -57,9 +77,8 @@ impl ShellSessionRegistry { .load(&info.target, &cache_root.join("artifacts")) .await?; let tools = bundle.verify_container_mount(&transport).await?; - Ok::<_, ToolRuntimeError>(Some(PreparedHome { - backend: ShellBackend::Container, - container_id: Some(runtime.container_id().into()), + Ok::<_, ToolRuntimeError>(Some(PreparedHome::Container { + container_id: runtime.container_id().into(), tools, })) } else if availability.local { @@ -67,11 +86,7 @@ impl ShellSessionRegistry { .load(&ToolTarget::native()?, &cache_root.join("artifacts")) .await?; let tools = bundle.prepare_local(&cache_root.join("runtimes")).await?; - Ok(Some(PreparedHome { - backend: ShellBackend::Local, - container_id: None, - tools, - })) + Ok(Some(PreparedHome::Local(tools))) } else { Ok(None) } @@ -106,7 +121,7 @@ impl ShellSessionRegistry { ssh.sort_by(|left, right| left.server.id().cmp(right.server.id())); let catalog = home .as_ref() - .map(|home| home.tools.catalog()) + .map(PreparedHome::catalog) .or_else(|| ssh.first().map(|ssh| ssh.tools.catalog())) .ok_or_else(|| { ToolRuntimeError::NotInitialized("no configured execution environment".into()) @@ -129,19 +144,16 @@ impl ShellSessionRegistry { .prepared_tools .as_ref() .ok_or_else(|| ToolRuntimeError::NotInitialized("execution environments".into()))?; - if prepared.home.as_ref().map(|home| home.backend) != self.resolve_local_backend().ok() { + if prepared.home.as_ref().map(PreparedHome::backend) != self.resolve_local_backend().ok() { return Err(ToolRuntimeError::EnvironmentChanged( PreparedEnvironment::HomeBackend, )); } - if let Some(container_id) = prepared - .home - .as_ref() - .and_then(|home| home.container_id.as_deref()) + if let Some(PreparedHome::Container { container_id, .. }) = prepared.home.as_ref() && self .container_runtime() .map(crate::ContainerShellRuntime::container_id) - != Some(container_id) + != Some(container_id.as_str()) { return Err(ToolRuntimeError::EnvironmentChanged( PreparedEnvironment::Container, @@ -166,22 +178,28 @@ impl ShellSessionRegistry { Ok(prepared) } - pub(crate) fn home_tools( + pub(crate) fn local_tools(&self) -> Result<&PreparedTools, ToolRuntimeError> { + match &self.prepared_environments()?.home { + Some(PreparedHome::Local(tools)) => Ok(tools), + _ => Err(ToolRuntimeError::NotInitialized("local environment".into())), + } + } + + pub(crate) fn container_tools( &self, - backend: ShellBackend, - ) -> Result<&PreparedTools, ToolRuntimeError> { - self.prepared_environments()? - .home - .as_ref() - .filter(|home| home.backend == backend) - .map(|home| &home.tools) - .ok_or_else(|| ToolRuntimeError::NotInitialized("home environment".into())) + ) -> Result<&PreparedTools, ToolRuntimeError> { + match &self.prepared_environments()?.home { + Some(PreparedHome::Container { tools, .. }) => Ok(tools), + _ => Err(ToolRuntimeError::NotInitialized( + "container environment".into(), + )), + } } pub(crate) fn ssh_tools( &self, server: &SshServer, - ) -> Result<(&SshRuntimeProfile, &PreparedTools), ToolRuntimeError> { + ) -> Result<(&SshRuntimeProfile, &PreparedTools), ToolRuntimeError> { self.prepared_environments()? .ssh .iter() @@ -201,33 +219,27 @@ impl ShellSessionRegistry { } #[cfg(test)] - pub(crate) fn with_test_tools(self, tools: PreparedTools) -> Self { - self.with_test_home(ShellBackend::Local, None, tools) + pub(crate) fn with_test_tools(self, tools: PreparedTools) -> Self { + self.with_test_home(PreparedHome::Local(tools)) } #[cfg(test)] pub(crate) fn with_test_container_tools( self, - tools: PreparedTools, + tools: PreparedTools, container_id: String, ) -> Self { - self.with_test_home(ShellBackend::Container, Some(container_id), tools) + self.with_test_home(PreparedHome::Container { + container_id, + tools, + }) } #[cfg(test)] - fn with_test_home( - mut self, - backend: ShellBackend, - container_id: Option, - tools: PreparedTools, - ) -> Self { - let catalog = tools.catalog().clone(); + fn with_test_home(mut self, home: PreparedHome) -> Self { + let catalog = home.catalog().clone(); self.prepared_tools = Some(PreparedToolEnvironments { - home: Some(PreparedHome { - backend, - container_id, - tools, - }), + home: Some(home), ssh: Vec::new(), catalog, }); @@ -236,7 +248,11 @@ impl ShellSessionRegistry { /// Records `server` as prepared with `tools`, as SSH bootstrap would. #[cfg(test)] - pub(crate) fn with_test_ssh(mut self, server: SshServer, tools: PreparedTools) -> Self { + pub(crate) fn with_test_ssh( + mut self, + server: SshServer, + tools: PreparedTools, + ) -> Self { let prepared = self .prepared_tools .as_mut() diff --git a/sandbox/src/tool_runtime.rs b/sandbox/src/tool_runtime.rs index e1ac5ba7..524026e8 100644 --- a/sandbox/src/tool_runtime.rs +++ b/sandbox/src/tool_runtime.rs @@ -6,6 +6,8 @@ use askama::Template; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; +use crate::ContainerPath; + /// Commands guaranteed by a prepared execution environment. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -226,7 +228,10 @@ impl ToolBundle { /// /// # Errors /// Returns errors for target mismatch, I/O failure or tampered cached content. - pub async fn prepare_local(&self, root: &Path) -> Result { + pub async fn prepare_local( + &self, + root: &Path, + ) -> Result, ToolRuntimeError> { if self.target != ToolTarget::native()? { return Err(ToolRuntimeError::TargetMismatch); } @@ -243,7 +248,7 @@ impl ToolBundle { pub async fn prepare_container_mount( &self, root: &Path, - ) -> Result<(crate::MountSpec, PreparedTools), ToolRuntimeError> { + ) -> Result<(crate::MountSpec, PreparedTools), ToolRuntimeError> { let bundle = self.clone(); let root = root.to_path_buf(); let source = blocking::unblock(move || bundle.prepare_local_sync(&root)).await?; @@ -259,8 +264,16 @@ impl ToolBundle { )) } - pub(crate) fn container_directory(&self) -> Result { - Ok(Path::new("/opt/aither/tools").join(self.cache_key()?)) + /// Where the prepared runtime is mounted inside a container. + pub(crate) fn container_directory(&self) -> Result { + Ok(ContainerPath::root() + .join("opt/aither/tools") + .join(self.cache_key()?)) + } + + /// The executable directory inside a container, which goes on its `PATH`. + pub(crate) fn container_bin_dir(&self) -> Result { + Ok(self.container_directory()?.join("bin")) } /// Returns the verified runtime at `/`, staging it when it is @@ -269,7 +282,7 @@ impl ToolBundle { /// Verifying a valid runtime takes no lock. Creation and replacement run under an /// exclusive lock on `/..lock`, so concurrent preparers never /// discard a runtime another preparer has just activated. - fn prepare_local_sync(&self, root: &Path) -> Result { + fn prepare_local_sync(&self, root: &Path) -> Result, ToolRuntimeError> { std::fs::create_dir_all(root)?; let root = std::fs::canonicalize(root)?; let key = self.cache_key()?; @@ -314,7 +327,10 @@ impl ToolBundle { } /// Verifies an existing runtime; `None` when it is absent or must be replaced. - fn reusable(&self, destination: &Path) -> Result, ToolRuntimeError> { + fn reusable( + &self, + destination: &Path, + ) -> Result>, ToolRuntimeError> { if !destination.try_exists()? { return Ok(None); } @@ -351,7 +367,10 @@ impl ToolBundle { /// Checks that every executable under `destination` is a regular, read-only /// executable file with the trusted digest. - fn verify_prepared(&self, destination: &Path) -> Result { + fn verify_prepared( + &self, + destination: &Path, + ) -> Result, ToolRuntimeError> { let bin_dir = destination.join("bin"); for binary in self.binaries.iter() { let path = bin_dir.join(binary.tool.executable_name(&self.target)); @@ -414,16 +433,20 @@ fn set_executable_readonly(path: &Path) -> Result<(), std::io::Error> { } /// Tool directory available only after successful initialization. +/// +/// `D` is the path type of the environment the tools run in: a host +/// [`PathBuf`] for the local sandbox and SSH, a [`ContainerPath`] inside a +/// container. #[derive(Debug, Clone)] -pub struct PreparedTools { - pub(crate) bin_dir: PathBuf, +pub struct PreparedTools { + pub(crate) bin_dir: D, pub(crate) catalog: ToolCatalog, } -impl PreparedTools { +impl PreparedTools { /// Read-only executable directory to expose to the sandbox. #[must_use] - pub const fn bin_dir(&self) -> &PathBuf { + pub const fn bin_dir(&self) -> &D { &self.bin_dir } @@ -432,7 +455,9 @@ impl PreparedTools { pub const fn catalog(&self) -> &ToolCatalog { &self.catalog } +} +impl PreparedTools { pub(crate) async fn verify_local_versions( &self, executor: E, @@ -531,20 +556,34 @@ struct ToolPathTemplate<'a> { script: &'a str, } -impl PreparedTools { - pub(crate) fn wrap_script(&self, script: &str) -> Result { - let directory = self - .bin_dir +impl PreparedTools { + /// The executable directory as text a shell script can name. + pub(crate) fn script_bin_dir(&self) -> Result<&str, ToolRuntimeError> { + self.bin_dir .to_str() - .ok_or_else(|| ToolRuntimeError::InvalidCache(self.bin_dir.clone()))?; - Ok(ToolPathTemplate { - bin_dir: shell_words::quote(directory), - script, - } - .render()?) + .ok_or_else(|| ToolRuntimeError::InvalidCache(self.bin_dir.clone())) + } + + pub(crate) fn wrap_script(&self, script: &str) -> Result { + render_tool_path(self.script_bin_dir()?, script) } } +impl PreparedTools { + pub(crate) fn wrap_script(&self, script: &str) -> Result { + render_tool_path(self.bin_dir.as_str(), script) + } +} + +/// Prepends `bin_dir` to a POSIX shell script's `PATH`. +fn render_tool_path(bin_dir: &str, script: &str) -> Result { + Ok(ToolPathTemplate { + bin_dir: shell_words::quote(bin_dir), + script, + } + .render()?) +} + /// Prepared command versions, in stable order independent of cache paths. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ToolCatalog { @@ -690,7 +729,7 @@ pub enum ToolRuntimeError { MountInfo(#[from] procfs_core::ProcError), #[doc = "The tools directory is not protected by a read-only container mount."] #[error("managed tools require a read-only container mount: {0}")] - MissingReadonlyMount(PathBuf), + MissingReadonlyMount(ContainerPath), /// Required command missing from a bundle. #[error("required tool is missing: {0:?}")] MissingTool(ManagedTool), @@ -778,6 +817,49 @@ pub mod tests { ToolBundle::new(ToolTarget::native().unwrap(), binaries).unwrap() } + /// Tools as a container sees them once the bundle is mounted read-only. + pub fn container_tools(bundle: &ToolBundle) -> PreparedTools { + PreparedTools { + bin_dir: bundle.container_bin_dir().unwrap(), + catalog: bundle.catalog(), + } + } + + // Runs on every host: Windows must still produce POSIX container paths. + #[test] + fn container_tool_paths_are_posix_on_every_host() { + let bundle = bundle(); + let key = bundle.cache_key().unwrap(); + assert_eq!( + bundle.container_directory().unwrap().as_str(), + format!("/opt/aither/tools/{key}") + ); + assert_eq!( + bundle.container_bin_dir().unwrap().as_str(), + format!("/opt/aither/tools/{key}/bin") + ); + let script = container_tools(&bundle) + .wrap_script("rg --version") + .unwrap(); + assert!( + script.starts_with(&format!("PATH=/opt/aither/tools/{key}/bin")), + "{script}" + ); + } + + #[tokio::test] + async fn container_mount_targets_the_posix_tool_directory() { + let cache = tempfile::tempdir().unwrap(); + let bundle = bundle(); + let (mount, host) = bundle.prepare_container_mount(cache.path()).await.unwrap(); + assert_eq!( + mount.container_path.as_str(), + format!("/opt/aither/tools/{}", bundle.cache_key().unwrap()) + ); + assert_eq!(Some(mount.host_path.as_path()), host.bin_dir().parent()); + assert!(mount.access.read_only()); + } + #[test] fn missing_baseline_tool_is_an_initialization_error() { let mut binaries = bundle().binaries.to_vec(); @@ -875,7 +957,7 @@ pub mod tests { )); } - fn assert_prepared(bundle: &ToolBundle, prepared: &PreparedTools) { + fn assert_prepared(bundle: &ToolBundle, prepared: &PreparedTools) { for tool in ManagedTool::ALL { let path = prepared .bin_dir() @@ -985,7 +1067,7 @@ pub mod tests { let cache = tempfile::tempdir().unwrap(); let bundle = bundle(); let ready = bundle.prepare_local(cache.path()).await.unwrap(); - let snapshot = |prepared: &PreparedTools| { + let snapshot = |prepared: &PreparedTools| { let mut entries = vec![prepared.bin_dir().parent().unwrap().to_path_buf()]; entries.push(prepared.bin_dir().clone()); entries.extend(ManagedTool::ALL.into_iter().map(|tool| { diff --git a/sandbox/src/tool_transport.rs b/sandbox/src/tool_transport.rs index 136c94fa..580c1f2d 100644 --- a/sandbox/src/tool_transport.rs +++ b/sandbox/src/tool_transport.rs @@ -6,6 +6,7 @@ use std::time::Duration; use askama::Template; use futures_lite::io::AsyncWriteExt; +use crate::ContainerPath; use crate::shell_session::{ContainerShellRuntime, SshServer}; use crate::tool_runtime::{PreparedTools, ToolBundle, ToolRuntimeError, ToolTarget}; @@ -178,6 +179,12 @@ struct ToolInstallTemplate<'a> { probe_only: bool, } +/// A container mount point as a POSIX path; `procfs` parses it into a host +/// [`PathBuf`], whose components follow host separator rules. +fn mount_point(mount: &procfs_core::process::MountInfo) -> &typed_path::UnixPath { + typed_path::UnixPath::new(mount.mount_point.as_os_str().as_encoded_bytes()) +} + #[derive(Template)] #[template(path = "tool_verify.sh", escape = "none")] struct ToolVerifyTemplate<'a> { @@ -189,24 +196,23 @@ impl ToolBundle { pub(crate) async fn verify_container_mount( &self, transport: &impl ToolTransport, - ) -> Result { + ) -> Result, ToolRuntimeError> { use procfs_core::FromRead; let directory = self.container_directory()?; let output = successful_output(transport.run("cat /proc/self/mountinfo", None).await?)?; let mounts = procfs_core::process::MountInfos::from_read(output.as_bytes())?; + let tools = typed_path::UnixPath::new(directory.as_str()); if !mounts .iter() - .any(|mount| mount.mount_point == directory && mount.mount_options.contains_key("ro")) + .any(|mount| mount_point(mount) == tools && mount.mount_options.contains_key("ro")) || mounts.iter().any(|mount| { - mount.mount_point.starts_with(&directory) && !mount.mount_options.contains_key("ro") + mount_point(mount).starts_with(tools) && !mount.mount_options.contains_key("ro") }) { return Err(ToolRuntimeError::MissingReadonlyMount(directory)); } let script = ToolVerifyTemplate { - directory: shell_words::quote( - directory.to_str().ok_or(ToolRuntimeError::InvalidProbe)?, - ), + directory: shell_words::quote(directory.as_str()), tools: self .binaries .iter() @@ -221,7 +227,7 @@ impl ToolBundle { let catalog = self.catalog(); catalog.verify_versions(&output)?; Ok(PreparedTools { - bin_dir: directory.join("bin"), + bin_dir: self.container_bin_dir()?, catalog, }) } @@ -235,7 +241,7 @@ impl ToolBundle { transport: &impl ToolTransport, root: &Path, authorize: F, - ) -> Result + ) -> Result, ToolRuntimeError> where F: FnOnce() -> Fut + Send, Fut: Future> + Send, @@ -360,7 +366,7 @@ mod tests { ] { let transport = MountTransport { mountinfo: MountInfoTemplate { - directory: directory.to_str().unwrap(), + directory: directory.as_str(), options, nested, nested_options, @@ -370,7 +376,10 @@ mod tests { }; let result = bundle.verify_container_mount(&transport).await; if accepted { - assert_eq!(result.unwrap().bin_dir(), &directory.join("bin")); + assert_eq!( + result.unwrap().bin_dir().as_str(), + format!("{directory}/bin") + ); } else { assert!(matches!( result,