From 8708b4b85018d6c6471bb9db545f9a30934cbeea Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Thu, 1 Oct 2026 22:40:42 -0700 Subject: [PATCH] refactor(runtime): decouple supervisor access and boundary audit validation Signed-off-by: Drew Newberry --- .../build-openshell-mxc-windows/SKILL.md | 8 + .../build-openshell-mxc-windows/reference.md | 6 +- crates/openshell-cli/src/ssh.rs | 3 + crates/openshell-driver-mxc/src/driver.rs | 2 +- crates/openshell-sandbox-backend/README.md | 13 + crates/openshell-sandbox-backend/src/audit.rs | 70 +++++ crates/openshell-sandbox-backend/src/lib.rs | 1 + .../openshell-sandbox-backend/src/runtime.rs | 28 +- .../src/delegated.rs | 290 ++++++++++++++---- .../openshell-supervisor-process/src/lib.rs | 2 + .../src/main_session.rs | 84 +++-- .../src/supervisor_session.rs | 46 ++- crates/openshell-supervisor/README.md | 17 + crates/openshell-supervisor/src/lib.rs | 126 +++++++- crates/openshell-supervisor/src/main.rs | 3 +- tasks/scripts/windows-msvc.ps1 | 2 +- 16 files changed, 575 insertions(+), 126 deletions(-) create mode 100644 crates/openshell-sandbox-backend/README.md create mode 100644 crates/openshell-sandbox-backend/src/audit.rs create mode 100644 crates/openshell-supervisor/README.md diff --git a/.agents/skills/build-openshell-mxc-windows/SKILL.md b/.agents/skills/build-openshell-mxc-windows/SKILL.md index a6f71388ff..678ca7d24e 100644 --- a/.agents/skills/build-openshell-mxc-windows/SKILL.md +++ b/.agents/skills/build-openshell-mxc-windows/SKILL.md @@ -21,6 +21,14 @@ Windows MSVC for the supported deliverables: It intentionally does not make Windows a Docker, Kubernetes, Podman, or VM runtime host. +The supervisor and supervisor-process libraries participate in native Windows +checks and tests. Their gateway session, boundary attachment, and TCP readiness +are portable; only the optional Unix SSH/readiness socket adapters are gated. +This is compile and control-plane coverage, not Windows isolation qualification. +Preserve readiness gating on authenticated gateway acceptance and reconnection. +Shared Sandbox Protocol audit validation defaults to strict Linux evidence; +concrete platform validators must be selected by the implementing backend. + ## Current Repository Shape The Windows build lane is implemented by these tracked files: diff --git a/.agents/skills/build-openshell-mxc-windows/reference.md b/.agents/skills/build-openshell-mxc-windows/reference.md index 81aecb9ec1..471eed009d 100644 --- a/.agents/skills/build-openshell-mxc-windows/reference.md +++ b/.agents/skills/build-openshell-mxc-windows/reference.md @@ -111,8 +111,6 @@ top-level workspace targets for check/test: --exclude openshell-driver-vault --exclude openshell-driver-vm --exclude openshell-sandbox ---exclude openshell-supervisor ---exclude openshell-supervisor-process --exclude openshell-vfio ``` @@ -124,6 +122,10 @@ egress proxy. The Kubernetes Secrets and Vault libraries still compile as gateway dependencies; only their standalone Unix-socket binaries and package-level tests are excluded as top-level targets. +The supervisor and supervisor-process packages now participate as top-level +native check/test targets. Their portable session, attachment, and TCP readiness +coverage does not enable a Windows isolation runtime. + ## Common Errors ### Unix imports leak into Windows builds diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index a7dc992e07..758143ba53 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -359,6 +359,7 @@ struct ConnectCancellation { } impl ConnectCancellation { + #[cfg_attr(not(unix), allow(clippy::unnecessary_wraps))] // Unix signal registration is fallible fn new() -> Result { Ok(Self { #[cfg(unix)] @@ -366,6 +367,7 @@ impl ConnectCancellation { }) } + #[cfg_attr(not(unix), allow(clippy::needless_pass_by_ref_mut))] // Unix receives through mutable signals async fn wait(&mut self, future: F) -> std::result::Result where F: Future, @@ -411,6 +413,7 @@ async fn terminate_and_reap_child(child: &mut Child, signal: Signal) -> Result Self { - let mut backend = Self::new(config); + let mut backend = Self::new("test", config); backend.invoker = WxcExecInvoker::mocked(&backend.config.wxc_exec_path); backend } diff --git a/crates/openshell-sandbox-backend/README.md b/crates/openshell-sandbox-backend/README.md new file mode 100644 index 0000000000..36d38e9a01 --- /dev/null +++ b/crates/openshell-sandbox-backend/README.md @@ -0,0 +1,13 @@ +# OpenShell Sandbox Protocol backend + +`OpenShellRuntimeBackend` implements the authenticated host side of the shared +Sandbox Protocol. + +Backend implementations may inject a `BoundaryAuditValidator` to interpret +their opaque confirmation evidence. The default Linux validator rejects +incomplete or foreign evidence. Confirmation compares the asserted properties +with the properties derived by the selected validator; validator injection does +not bypass generation, session, resource, identity, or outer-fence checks. + +Concrete platform validators belong to the implementing backend, not this +shared transport library. diff --git a/crates/openshell-sandbox-backend/src/audit.rs b/crates/openshell-sandbox-backend/src/audit.rs new file mode 100644 index 0000000000..59f6ba1dc7 --- /dev/null +++ b/crates/openshell-sandbox-backend/src/audit.rs @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Backend-selected interpretation of opaque boundary confirmation evidence. + +use openshell_isolation_interface::contract::{BackendError, BoundaryProperties}; + +/// Validate measured evidence and derive the properties it actually supports. +/// Implementations must reject incomplete evidence and unsupported formats. +pub trait BoundaryAuditValidator: std::fmt::Debug + Send + Sync { + /// # Errors + /// Returns an error when evidence cannot establish the boundary guarantees. + fn validate(&self, evidence: &serde_json::Value) -> Result; +} + +/// Default evidence interpreter for the Linux `OpenShell` sandbox. +#[derive(Debug)] +pub struct LinuxBoundaryAuditValidator; + +impl BoundaryAuditValidator for LinuxBoundaryAuditValidator { + fn validate(&self, evidence: &serde_json::Value) -> Result { + let audit: crate::boundary_protocol::NativeLinuxSandboxAuditEvidence = + serde_json::from_value(evidence.clone()).map_err(|error| { + BackendError::Confirm(format!("decode Linux sandbox audit evidence: {error}")) + })?; + audit.validate()?; + Ok(audit.properties()) + } +} + +#[cfg(test)] +mod tests { + use super::{BoundaryAuditValidator as _, LinuxBoundaryAuditValidator}; + + #[test] + fn default_validator_accepts_complete_linux_evidence() { + let evidence = serde_json::json!({ + "capabilities": {"inheritable": 0, "permitted": 0, "effective": 0, + "bounding": 0, "ambient": 0}, + "no_new_privileges": true, "sandbox_dumpable": false, + "child_dumpable": true, "core_limit_zero": true, + "native_architecture": "test", "kernel_release": "test", + "seccomp": { + "new_listener": true, "notification_round_trip": true, + "id_validation": true, "addfd_send": true, + "retained_socket_operation": true, "proc_fd_identity": true, + "task_memory_read": true, "task_memory_write": true, "cancellation": true, + "task_memory_writes_disabled": false + }, + "landlock_abi": 3, "landlock_allow_deny": true, + "udp_dns_round_trip": true, "tcp_dns_round_trip": true, + "tcp_allow_round_trip": true, "tcp_deny_round_trip": true + }); + let properties = LinuxBoundaryAuditValidator.validate(&evidence).unwrap(); + let audit: crate::boundary_protocol::NativeLinuxSandboxAuditEvidence = + serde_json::from_value(evidence.clone()).unwrap(); + assert_eq!(properties, audit.properties()); + let mut incomplete = evidence; + incomplete["no_new_privileges"] = false.into(); + assert!(LinuxBoundaryAuditValidator.validate(&incomplete).is_err()); + } + + #[test] + fn default_validator_rejects_unknown_platform_and_incomplete_evidence() { + for platform in ["windows_mxc", "unknown", "linux"] { + let evidence = serde_json::json!({"platform": platform, "evidence": {}}); + assert!(LinuxBoundaryAuditValidator.validate(&evidence).is_err()); + } + } +} diff --git a/crates/openshell-sandbox-backend/src/lib.rs b/crates/openshell-sandbox-backend/src/lib.rs index 87b25dccd3..62ddf75e3c 100644 --- a/crates/openshell-sandbox-backend/src/lib.rs +++ b/crates/openshell-sandbox-backend/src/lib.rs @@ -8,6 +8,7 @@ //! runtime serves the same protocol using the generated server and shared wire //! types in this crate. +pub mod audit; pub mod boundary_protocol; pub mod mediation; mod runtime; diff --git a/crates/openshell-sandbox-backend/src/runtime.rs b/crates/openshell-sandbox-backend/src/runtime.rs index 641d50cd24..e015a95814 100644 --- a/crates/openshell-sandbox-backend/src/runtime.rs +++ b/crates/openshell-sandbox-backend/src/runtime.rs @@ -73,6 +73,7 @@ fn begin_recovery_window( /// Host-side `OpenShell` Sandbox Protocol implementation registered with the supervisor. #[derive(Debug)] pub struct OpenShellRuntimeBackend { + audit_validator: Arc, ca_file_paths: Arc>>, provider_credentials: openshell_core::provider_credentials::ProviderCredentialState, sandbox_bearer: openshell_core::jwt::SessionBearerTokenSlot, @@ -99,11 +100,22 @@ impl OpenShellRuntimeBackend { sandbox_bearer: openshell_core::jwt::SessionBearerTokenSlot, ) -> Self { Self { + audit_validator: Arc::new(crate::audit::LinuxBoundaryAuditValidator), ca_file_paths, provider_credentials, sandbox_bearer, } } + + /// Select the backend implementation that validates opaque audit evidence. + #[must_use] + pub fn with_audit_validator( + mut self, + validator: Arc, + ) -> Self { + self.audit_validator = validator; + self + } } #[async_trait] @@ -147,6 +159,7 @@ impl IsolationBackend for OpenShellRuntimeBackend { )); } Ok(Box::new(RemoteBound { + audit_validator: self.audit_validator.clone(), client: client.clone(), agent: sandbox.agent, policy: sandbox.policy, @@ -292,6 +305,7 @@ fn validate_control_port(port: u32) -> Result<(), BackendError> { } struct RemoteBound { + audit_validator: Arc, client: Arc, agent: AgentSpec, policy: openshell_core::policy::SandboxPolicy, @@ -332,17 +346,10 @@ impl BoundBoundary for RemoteBound { .to_string(), )); } - let audit: crate::boundary_protocol::NativeLinuxSandboxAuditEvidence = - serde_json::from_value(confirmation.backend_audit.clone()).map_err(|error| { - BackendError::Confirm(format!( - "decode native Linux sandbox audit evidence: {error}" - )) - })?; - audit.validate()?; - if confirmation.properties != audit.properties() { + let properties = self.audit_validator.validate(&confirmation.backend_audit)?; + if confirmation.properties != properties { return Err(BackendError::Confirm( - "sandbox confirmation properties do not match native Linux audit evidence" - .to_string(), + "sandbox confirmation properties do not match validated audit evidence".to_string(), )); } let client = self.client.clone(); @@ -2958,6 +2965,7 @@ mod tests { test_bearer(&expected_token), )); let bound = RemoteBound { + audit_validator: Arc::new(crate::audit::LinuxBoundaryAuditValidator), client: client.clone(), agent: context.agent, policy: context.policy, diff --git a/crates/openshell-supervisor-process/src/delegated.rs b/crates/openshell-supervisor-process/src/delegated.rs index 87101f62a2..093be82f39 100644 --- a/crates/openshell-supervisor-process/src/delegated.rs +++ b/crates/openshell-supervisor-process/src/delegated.rs @@ -11,8 +11,10 @@ use miette::Result; use openshell_isolation_interface::contract::{ BoundaryExec, BoundaryLoopbackConnector, BoundaryProcess, }; +#[cfg(unix)] use openshell_ocsf::{ActivityId, AppLifecycleBuilder, SeverityId, StatusId, ocsf_emit}; +#[cfg(unix)] fn ocsf_ctx() -> &'static openshell_ocsf::EventContext { openshell_ocsf::ctx::ctx() } @@ -90,76 +92,31 @@ pub async fn start_boundary_access( ) -> Result { let instance_id = uuid::Uuid::new_v4().to_string(); let terminating = Arc::new(AtomicBool::new(false)); - let Some(ssh_socket_path) = ssh_socket_path.map(std::path::PathBuf::from) else { - return Ok(BoundaryAccess { - instance_id, - terminating, - ssh_task: None, - session_task: None, - session_readiness: None, - main_session: None, - }); - }; - let attachment = agent .attach() .await .map_err(|error| miette::miette!(error.to_string()))?; let main_session = crate::main_session::MainSession::from_boundary(attachment, agent); + let ssh_socket_path = ssh_socket_path.map(std::path::PathBuf::from); + let ssh_task = start_optional_ssh( + ssh_socket_path.clone(), + shared_ssh_socket, + ca_file_paths, + boundary_exec, + port_forward.clone(), + main_session.clone(), + ) + .await?; - let (ssh_ready_tx, ssh_ready_rx) = tokio::sync::oneshot::channel(); - let listen_path = ssh_socket_path.clone(); - let ssh_port_forward = port_forward.clone(); - let ssh_main_session = main_session.clone(); - let ssh_task = tokio::spawn(async move { - if let Err(error) = crate::ssh::run_ssh_server( - listen_path, - ssh_ready_tx, - ca_file_paths, - shared_ssh_socket, - ssh_port_forward, - boundary_exec, - Some(ssh_main_session), - ) - .await - { - ocsf_emit!( - AppLifecycleBuilder::new(ocsf_ctx()) - .activity(ActivityId::Fail) - .severity(SeverityId::Critical) - .status(StatusId::Failure) - .message(format!("SSH server failed: {error}")) - .build() - ); - } - }); - - match tokio::time::timeout(Duration::from_secs(10), ssh_ready_rx).await { - Ok(Ok(Ok(()))) => {} - Ok(Ok(Err(error))) => { - ssh_task.abort(); - return Err(error.context("SSH server failed during startup")); - } - Ok(Err(_)) => { - ssh_task.abort(); - return Err(miette::miette!( - "SSH server task ended before signaling readiness" - )); - } - Err(_) => { - ssh_task.abort(); - return Err(miette::miette!( - "SSH server did not start within 10 seconds" - )); - } - } - + // Gateway authentication, forwarding, and readiness do not depend on SSH + // or the host OS. Acceptance remains false until the gateway authenticates + // this supervisor, and reconnects retain main's retry behavior. let (session_task, session_readiness) = match (openshell_endpoint, sandbox_id) { (Some(endpoint), Some(id)) => { let (task, accepted) = crate::supervisor_session::spawn_with_readiness( endpoint.to_string(), id.to_string(), - ssh_socket_path, + ssh_socket_path.unwrap_or_default(), port_forward, None, terminating.clone(), @@ -168,24 +125,91 @@ pub async fn start_boundary_access( session_id_updates: supervisor_session_updates, }, ); - // Session establishment retries through gateway restarts. The - // readiness socket remains absent until the gateway accepts the - // session, so a transient delay cannot kill the supervisor. (Some(task), Some(accepted)) } _ => (None, None), }; - Ok(BoundaryAccess { instance_id, terminating, - ssh_task: Some(ssh_task), + ssh_task, session_task, session_readiness, main_session: Some(main_session), }) } +/// The optional SSH adapter is the only platform-specific access component. +#[cfg(unix)] +async fn start_optional_ssh( + socket_path: Option, + shared_socket: bool, + ca_file_paths: Option<(std::path::PathBuf, std::path::PathBuf)>, + boundary_exec: Arc, + port_forward: Arc, + main_session: Arc, +) -> Result>> { + let Some(socket_path) = socket_path else { + return Ok(None); + }; + let (ready_tx, ready_rx) = tokio::sync::oneshot::channel(); + let task = tokio::spawn(async move { + if let Err(error) = crate::ssh::run_ssh_server( + socket_path, + ready_tx, + ca_file_paths, + shared_socket, + port_forward, + boundary_exec, + Some(main_session), + ) + .await + { + ocsf_emit!( + AppLifecycleBuilder::new(ocsf_ctx()) + .activity(ActivityId::Fail) + .severity(SeverityId::Critical) + .status(StatusId::Failure) + .message(format!("SSH server failed: {error}")) + .build() + ); + } + }); + match tokio::time::timeout(Duration::from_secs(10), ready_rx).await { + Ok(Ok(Ok(()))) => Ok(Some(task)), + result => { + task.abort(); + match result { + Ok(Ok(Err(error))) => Err(error.context("SSH server failed during startup")), + Ok(Err(_)) => Err(miette::miette!( + "SSH server task ended before signaling readiness" + )), + Err(_) => Err(miette::miette!( + "SSH server did not start within 10 seconds" + )), + Ok(Ok(Ok(()))) => unreachable!(), + } + } + } +} + +#[cfg(not(unix))] +async fn start_optional_ssh( + socket_path: Option, + _shared_socket: bool, + _ca_file_paths: Option<(std::path::PathBuf, std::path::PathBuf)>, + _boundary_exec: Arc, + _port_forward: Arc, + _main_session: Arc, +) -> Result>> { + if socket_path.is_some() { + return Err(miette::miette!( + "SSH access sockets are unsupported on this host" + )); + } + Ok(None) +} + /// Report the canonical process exit until the gateway acknowledges it. pub async fn report_main_process_exit( endpoint: &str, @@ -238,6 +262,144 @@ pub async fn finalize_main_process_exit(endpoint: &str, sandbox_id: &str, instan mod tests { use super::*; + // Unit-only boundary fixture: these tests exercise access-plane composition, + // not isolation enforcement or E2E qualification. + struct AccessBoundary; + + #[async_trait::async_trait] + impl BoundaryProcess for AccessBoundary { + async fn attach( + &self, + ) -> std::result::Result< + openshell_isolation_interface::contract::ProcessAttachment, + openshell_isolation_interface::contract::BackendError, + > { + Ok(openshell_isolation_interface::contract::ProcessAttachment { + stdin: Box::new(tokio::io::sink()), + stdout: Box::new(tokio::io::empty()), + stderr: Some(Box::new(tokio::io::empty())), + terminal: None, + }) + } + async fn wait( + &self, + ) -> std::result::Result< + openshell_isolation_interface::contract::BoundaryExitStatus, + openshell_isolation_interface::contract::BackendError, + > { + Ok(openshell_isolation_interface::contract::BoundaryExitStatus::Exited(7)) + } + async fn signal( + &self, + _: openshell_isolation_interface::contract::BoundarySignal, + ) -> std::result::Result<(), openshell_isolation_interface::contract::BackendError> + { + Ok(()) + } + async fn terminate( + &self, + ) -> std::result::Result<(), openshell_isolation_interface::contract::BackendError> + { + Ok(()) + } + } + + #[async_trait::async_trait] + impl BoundaryExec for AccessBoundary { + async fn exec( + &self, + _: openshell_isolation_interface::contract::ExecSpec, + ) -> std::result::Result< + openshell_isolation_interface::contract::ExecSession, + openshell_isolation_interface::contract::BackendError, + > { + Err( + openshell_isolation_interface::contract::BackendError::Unsupported( + "unit fixture has no exec".into(), + ), + ) + } + } + + #[async_trait::async_trait] + impl BoundaryLoopbackConnector for AccessBoundary { + async fn connect( + &self, + _: openshell_isolation_interface::contract::LoopbackTarget, + ) -> std::result::Result< + openshell_isolation_interface::contract::BoundaryDuplexStream, + openshell_isolation_interface::contract::BackendError, + > { + Err( + openshell_isolation_interface::contract::BackendError::Unsupported( + "unit fixture has no forwarding".into(), + ), + ) + } + } + + #[tokio::test] + async fn no_ssh_access_retains_main_attachment_and_session_readiness() { + let boundary = Arc::new(AccessBoundary); + let access = start_boundary_access( + Some("sandbox"), + Some("http://127.0.0.1:1"), + None, + false, + None, + boundary.clone(), + boundary.clone(), + boundary, + None, + ) + .await + .expect("access without SSH is portable"); + assert!(access.ssh_task.is_none()); + assert!(access.session_task.is_some()); + assert!( + !*access + .session_readiness() + .expect("readiness exists") + .borrow() + ); + let main = access + .main_session + .as_ref() + .expect("main attachment retained"); + access.publish_main_exit(7, true).await; + main.begin_terminal_attachment() + .expect("fast-exit attachment survives without SSH"); + main.end_terminal_attachment(); + let terminating = access.terminating.clone(); + drop(access); + assert!(terminating.load(Ordering::Acquire)); + } + + #[cfg(not(unix))] + #[tokio::test] + async fn explicit_unix_ssh_socket_is_rejected() { + let boundary = Arc::new(AccessBoundary); + let result = start_boundary_access( + None, + None, + Some("health.sock"), + false, + None, + boundary.clone(), + boundary.clone(), + boundary, + None, + ) + .await; + assert!( + result + .err() + .expect("Unix SSH unsupported") + .to_string() + .contains("unsupported") + ); + } + #[tokio::test] async fn expected_post_exit_attachment_is_preserved_for_remote_main() { let main_session = crate::main_session::MainSession::inert(); diff --git a/crates/openshell-supervisor-process/src/lib.rs b/crates/openshell-supervisor-process/src/lib.rs index 023a6c8e73..df0f21a519 100644 --- a/crates/openshell-supervisor-process/src/lib.rs +++ b/crates/openshell-supervisor-process/src/lib.rs @@ -12,7 +12,9 @@ pub mod delegated; pub mod log_push; pub mod main_session; pub mod skills; +#[cfg(unix)] pub mod ssh; pub mod supervisor_session; +#[cfg(unix)] mod unix_socket; diff --git a/crates/openshell-supervisor-process/src/main_session.rs b/crates/openshell-supervisor-process/src/main_session.rs index 1e59aa5c6e..ba0685ce1a 100644 --- a/crates/openshell-supervisor-process/src/main_session.rs +++ b/crates/openshell-supervisor-process/src/main_session.rs @@ -4,14 +4,19 @@ //! Retained I/O multiplexer for the canonical sandbox process. use std::collections::VecDeque; +#[cfg(unix)] use std::io::{Read, Write}; +#[cfg(unix)] use std::os::fd::AsRawFd; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use bytes::Bytes; +#[cfg(unix)] use nix::fcntl::{FcntlArg, OFlag, fcntl}; +#[cfg(unix)] use nix::pty::Winsize; +#[cfg(unix)] use tokio::io::unix::AsyncFd; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::sync::Notify; @@ -24,6 +29,7 @@ use openshell_isolation_interface::contract::{ const OUTPUT_BUFFER_BYTES: usize = 1024 * 1024; /// Canonical-process I/O retained by the supervisor session multiplexer. +#[cfg(unix)] pub enum ProcessIo { Pty(std::fs::File), Pipes { @@ -191,12 +197,14 @@ impl MainOutputCursor { } pub struct MainSession { + #[cfg(unix)] pid: u32, terminal: bool, input: tokio::sync::mpsc::Sender>, output: Arc, input_owner: Mutex>, next_owner: AtomicU64, + #[cfg(unix)] pty_master: Option>, boundary_process: Option>, boundary_terminal: Option>, @@ -218,12 +226,14 @@ impl MainSession { pub fn inert_with_input() -> (Arc, tokio::sync::mpsc::Receiver>) { let (input, input_rx) = tokio::sync::mpsc::channel(64); let session = Arc::new(Self { + #[cfg(unix)] pid: 1, terminal: false, input, output: OutputLog::new(), input_owner: Mutex::new(None), next_owner: AtomicU64::new(1), + #[cfg(unix)] pty_master: None, boundary_process: None, boundary_terminal: None, @@ -240,7 +250,7 @@ impl MainSession { (session, input_rx) } - #[cfg(test)] + #[cfg(all(test, unix))] pub fn terminal_for_test() -> (Arc, std::fs::File) { let pty = nix::pty::openpty(None, None).expect("open test PTY"); let slave = std::fs::File::from(pty.slave); @@ -250,7 +260,7 @@ impl MainSession { ) } - #[cfg(test)] + #[cfg(all(test, unix))] #[allow(unsafe_code)] pub fn terminal_size_for_test(&self) -> (u16, u16) { let master = self.pty_master.as_ref().expect("terminal PTY master"); @@ -261,6 +271,7 @@ impl MainSession { } #[must_use] + #[cfg(unix)] pub fn new(io: ProcessIo, pid: u32) -> Arc { let terminal = matches!(io, ProcessIo::Pty(_)); let (input, input_rx) = tokio::sync::mpsc::channel::>(64); @@ -278,6 +289,7 @@ impl MainSession { output: OutputLog::new(), input_owner: Mutex::new(None), next_owner: AtomicU64::new(1), + #[cfg(unix)] pty_master, boundary_process: None, boundary_terminal: None, @@ -312,12 +324,14 @@ impl MainSession { let terminal_mode = terminal.is_some(); let (input, mut input_rx) = tokio::sync::mpsc::channel::>(64); let session = Arc::new(Self { + #[cfg(unix)] pid: 0, terminal: terminal_mode, input, output: OutputLog::new(), input_owner: Mutex::new(None), next_owner: AtomicU64::new(1), + #[cfg(unix)] pty_master: None, boundary_process: Some(process), boundary_terminal: terminal, @@ -370,6 +384,7 @@ impl MainSession { session } + #[cfg(unix)] fn start_io( this: &Arc, io: ProcessIo, @@ -655,38 +670,65 @@ impl MainSession { .await; return; } - let Some(master) = self.pty_master.as_ref() else { - return; - }; - let winsize = Winsize { - ws_row: u16::try_from(rows.max(1)).unwrap_or(u16::MAX), - ws_col: u16::try_from(columns.max(1)).unwrap_or(u16::MAX), - ws_xpixel: u16::try_from(pixel_width).unwrap_or(u16::MAX), - ws_ypixel: u16::try_from(pixel_height).unwrap_or(u16::MAX), - }; - #[allow(unsafe_code)] - unsafe { - libc::ioctl(master.as_raw_fd(), libc::TIOCSWINSZ, &winsize); + #[cfg(not(unix))] + let _ = (columns, rows, pixel_width, pixel_height); + #[cfg(unix)] + { + let Some(master) = self.pty_master.as_ref() else { + return; + }; + let winsize = Winsize { + ws_row: u16::try_from(rows.max(1)).unwrap_or(u16::MAX), + ws_col: u16::try_from(columns.max(1)).unwrap_or(u16::MAX), + ws_xpixel: u16::try_from(pixel_width).unwrap_or(u16::MAX), + ws_ypixel: u16::try_from(pixel_height).unwrap_or(u16::MAX), + }; + #[allow(unsafe_code)] + unsafe { + libc::ioctl(master.as_raw_fd(), libc::TIOCSWINSZ, &winsize); + } } } + /// Preserve the local Unix signal surface, including SIGQUIT. + #[cfg(unix)] pub async fn signal_group(&self, signal: nix::sys::signal::Signal) -> Result<(), String> { - if let Some(process) = self.boundary_process.as_ref() { - let signal = match signal { + if self.boundary_process.is_some() { + let boundary_signal = match signal { nix::sys::signal::Signal::SIGHUP => BoundarySignal::Hup, nix::sys::signal::Signal::SIGINT => BoundarySignal::Int, nix::sys::signal::Signal::SIGKILL => BoundarySignal::Kill, nix::sys::signal::Signal::SIGTERM => BoundarySignal::Term, other => return Err(format!("boundary signal {other:?} is unsupported")), }; + return self.signal_boundary_group(boundary_signal).await; + } + let pid = i32::try_from(self.pid).unwrap_or(i32::MAX); + nix::sys::signal::kill(nix::unistd::Pid::from_raw(-pid), signal) + .map_err(|error| error.to_string()) + } + + pub async fn signal_boundary_group(&self, signal: BoundarySignal) -> Result<(), String> { + if let Some(process) = self.boundary_process.as_ref() { return process .signal(signal) .await .map_err(|error| error.to_string()); } - let pid = i32::try_from(self.pid).unwrap_or(i32::MAX); - nix::sys::signal::kill(nix::unistd::Pid::from_raw(-pid), signal) - .map_err(|error| error.to_string()) + #[cfg(unix)] + { + let pid = i32::try_from(self.pid).unwrap_or(i32::MAX); + let signal = match signal { + BoundarySignal::Hup => nix::sys::signal::Signal::SIGHUP, + BoundarySignal::Int => nix::sys::signal::Signal::SIGINT, + BoundarySignal::Kill => nix::sys::signal::Signal::SIGKILL, + BoundarySignal::Term => nix::sys::signal::Signal::SIGTERM, + }; + nix::sys::signal::kill(nix::unistd::Pid::from_raw(-pid), signal) + .map_err(|error| error.to_string()) + } + #[cfg(not(unix))] + Err("local process-group signaling is unsupported on Windows".to_string()) } #[must_use] @@ -700,6 +742,7 @@ impl MainSession { } } +#[cfg(unix)] fn set_nonblocking(file: &std::fs::File) -> Result<(), nix::errno::Errno> { let flags = fcntl(file.as_raw_fd(), FcntlArg::F_GETFL)?; let flags = OFlag::from_bits_truncate(flags); @@ -785,7 +828,7 @@ mod tests { session.resize(120, 40, 0, 0).await; assert_eq!(*terminal.size.lock().unwrap(), Some((120, 40))); session - .signal_group(nix::sys::signal::Signal::SIGINT) + .signal_boundary_group(BoundarySignal::Int) .await .unwrap(); assert_eq!(*process.signals.lock().unwrap(), vec![BoundarySignal::Int]); @@ -955,6 +998,7 @@ mod tests { )); } + #[cfg(unix)] #[tokio::test] async fn terminal_pump_reads_output_and_writes_input() { let (session, mut slave) = MainSession::terminal_for_test(); diff --git a/crates/openshell-supervisor-process/src/supervisor_session.rs b/crates/openshell-supervisor-process/src/supervisor_session.rs index 5be01017eb..4802f03c46 100644 --- a/crates/openshell-supervisor-process/src/supervisor_session.rs +++ b/crates/openshell-supervisor-process/src/supervisor_session.rs @@ -522,7 +522,7 @@ pub async fn report_main_process_exit( Ok(()) } -#[cfg(test)] +#[cfg(all(test, unix))] pub(crate) async fn test_bridge_ssh_relay( target: tokio::net::UnixStream, inbound: mpsc::Receiver>, @@ -822,22 +822,32 @@ async fn open_target( port_forward: &Arc, expected_ssh_peer_pid: Option, ) -> Result, Box> { + #[cfg(not(unix))] + let _ = (ssh_socket_path, expected_ssh_peer_pid); match relay_open.target.as_ref() { Some(relay_open::Target::Tcp(target)) => open_tcp_target(target, port_forward).await, Some(relay_open::Target::Ssh(_)) | None => { - let runtime_path = crate::unix_socket::runtime_path(ssh_socket_path); - let stream = tokio::net::UnixStream::connect(runtime_path.as_ref()).await?; - if let Some(expected_pid) = expected_ssh_peer_pid { - let credentials = stream.peer_cred()?; - let actual_pid = credentials.pid().and_then(|pid| u32::try_from(pid).ok()); - if actual_pid != Some(expected_pid) { - return Err(format!( + if ssh_socket_path.as_os_str().is_empty() { + return Err("SSH access is not configured for this supervisor".into()); + } + #[cfg(not(unix))] + return Err("SSH relay targets are unsupported by the Windows supervisor".into()); + #[cfg(unix)] + { + let runtime_path = crate::unix_socket::runtime_path(ssh_socket_path); + let stream = tokio::net::UnixStream::connect(runtime_path.as_ref()).await?; + if let Some(expected_pid) = expected_ssh_peer_pid { + let credentials = stream.peer_cred()?; + let actual_pid = credentials.pid().and_then(|pid| u32::try_from(pid).ok()); + if actual_pid != Some(expected_pid) { + return Err(format!( "SSH relay peer PID mismatch: expected {expected_pid}, got {actual_pid:?}" ) .into()); + } } + Ok(Box::new(stream)) } - Ok(Box::new(stream)) } } } @@ -945,10 +955,8 @@ mod target_tests { mod ocsf_event_tests { use super::*; - #[cfg(target_os = "linux")] struct UnusedLoopbackConnector; - #[cfg(target_os = "linux")] #[async_trait::async_trait] impl BoundaryLoopbackConnector for UnusedLoopbackConnector { async fn connect( @@ -975,6 +983,22 @@ mod ocsf_event_tests { } } + #[tokio::test] + async fn ssh_relay_without_adapter_is_rejected_on_every_host() { + let connector: Arc = Arc::new(UnusedLoopbackConnector); + let result = open_target( + &ssh_relay_open("no-ssh"), + std::path::Path::new(""), + &connector, + None, + ) + .await; + let error = result + .err() + .expect("missing SSH adapter must fail explicitly"); + assert!(error.to_string().contains("not configured")); + } + #[test] fn gateway_endpoint_parses_https_with_port() { let e = ocsf_gateway_endpoint("https://gateway.openshell:8443"); diff --git a/crates/openshell-supervisor/README.md b/crates/openshell-supervisor/README.md new file mode 100644 index 0000000000..07f87b8546 --- /dev/null +++ b/crates/openshell-supervisor/README.md @@ -0,0 +1,17 @@ +# OpenShell supervisor + +The supervisor consumes the shared isolation backend contract. Its gateway +session, canonical process attachment, and TCP readiness do not require SSH or +a Unix host. + +TCP readiness opens only after the gateway accepts the authenticated session. +Session loss closes readiness; accepted reconnection restores it. Dropping the +readiness guard closes the listener. A requested Unix readiness endpoint fails +explicitly on unsupported hosts, even before session acceptance. + +The optional Unix SSH adapter remains separate from boundary-based process I/O. +Local Unix process signaling retains its existing signal surface, including +`SIGQUIT`. Remote signals use the backend-neutral `BoundarySignal` contract. + +These portable control-plane foundations do not qualify a platform isolation +backend or enable a Windows workload runtime. diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 99607dbedb..869ef888cb 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -100,21 +100,39 @@ enum ReadinessEndpoint { } enum ReadinessListener { + #[cfg(unix)] Unix(tokio::net::UnixListener), Tcp(tokio::net::TcpListener), } impl ReadinessEndpoint { + fn prepare(&self) -> Result<()> { + match self { + Self::Unix(path) => prepare_control_readiness_path(path), + Self::Tcp(_) => Ok(()), + } + } + fn bind(&self) -> Result { match self { Self::Unix(path) => { - prepare_control_readiness_path(path)?; - tokio::net::UnixListener::bind(path) - .map(ReadinessListener::Unix) - .into_diagnostic() - .wrap_err_with(|| { - format!("bind supervisor readiness socket on {}", path.display()) - }) + #[cfg(not(unix))] + { + let _ = path; + Err(miette::miette!( + "Unix readiness sockets are unsupported on this host" + )) + } + #[cfg(unix)] + { + prepare_control_readiness_path(path)?; + tokio::net::UnixListener::bind(path) + .map(ReadinessListener::Unix) + .into_diagnostic() + .wrap_err_with(|| { + format!("bind supervisor readiness socket on {}", path.display()) + }) + } } Self::Tcp(port) => bind_readiness_tcp(*port) .and_then(tokio::net::TcpListener::from_std) @@ -126,7 +144,10 @@ impl ReadinessEndpoint { fn remove(&self) { if let Self::Unix(path) = self { + #[cfg(unix)] let _ = std::fs::remove_file(path); + #[cfg(not(unix))] + let _ = path; // Unix endpoints are rejected before binding on this host. } } } @@ -153,6 +174,7 @@ fn bind_readiness_tcp(port: u16) -> std::io::Result { impl ReadinessListener { async fn accept(&self) -> std::io::Result<()> { match self { + #[cfg(unix)] Self::Unix(listener) => listener.accept().await.map(drop), Self::Tcp(listener) => listener.accept().await.map(drop), } @@ -169,9 +191,7 @@ impl ControlReadiness { endpoint: ReadinessEndpoint, mut session_readiness: Option>, ) -> Result { - if let ReadinessEndpoint::Unix(path) = &endpoint { - prepare_control_readiness_path(path)?; - } + endpoint.prepare()?; let listener = if session_readiness .as_ref() .is_some_and(|readiness| !*readiness.borrow()) @@ -281,6 +301,13 @@ fn prepare_control_readiness_path(path: &std::path::Path) -> Result<()> { Ok(()) } +#[cfg(not(unix))] +fn prepare_control_readiness_path(_path: &std::path::Path) -> Result<()> { + Err(miette::miette!( + "Unix readiness sockets are unsupported on this host" + )) +} + impl Drop for ControlReadiness { fn drop(&mut self) { self.task.abort(); @@ -1338,7 +1365,11 @@ fn persist_main_exit_marker(path: &std::path::Path, exit_code: i32) -> std::io:: writeln!(file, "exit_code={exit_code}")?; file.sync_all()?; std::fs::rename(&temporary, path)?; - std::fs::File::open(parent)?.sync_all() + // Windows cannot open directories through File::open. The marker data is + // flushed above and rename still atomically replaces the previous value. + #[cfg(unix)] + std::fs::File::open(parent)?.sync_all()?; + Ok(()) } /// Flush aggregated denial summaries to the gateway via `SubmitPolicyAnalysis`. @@ -4973,6 +5004,7 @@ mod tests { assert!(prepare_network_proxy_tls_dir(Some(writable)).is_err()); } + #[cfg(unix)] #[tokio::test] async fn control_readiness_exists_only_while_guard_is_live() { let root = tempfile::tempdir().unwrap(); @@ -4986,6 +5018,7 @@ mod tests { assert!(check_control_readiness(&path).is_err()); } + #[cfg(unix)] #[tokio::test] async fn control_readiness_tracks_supervisor_session() { let root = tempfile::tempdir().unwrap(); @@ -5052,12 +5085,19 @@ mod tests { let (session_tx, session_rx) = tokio::sync::watch::channel(true); let readiness = ControlReadiness::start(ReadinessEndpoint::Tcp(port), Some(session_rx)) .expect("start TCP readiness listener"); - let connects = || std::net::TcpStream::connect(("127.0.0.1", port)).is_ok(); - assert!(connects(), "accepted session is ready"); + let connects = || async { + timeout( + Duration::from_millis(100), + tokio::net::TcpStream::connect(("127.0.0.1", port)), + ) + .await + .is_ok_and(|result| result.is_ok()) + }; + assert!(connects().await, "accepted session is ready"); session_tx.send_replace(false); timeout(Duration::from_secs(1), async { - while connects() { + while connects().await { tokio::task::yield_now().await; } }) @@ -5066,7 +5106,7 @@ mod tests { session_tx.send_replace(true); timeout(Duration::from_secs(1), async { - while !connects() { + while !connects().await { tokio::task::yield_now().await; } }) @@ -5075,7 +5115,7 @@ mod tests { drop(readiness); timeout(Duration::from_secs(1), async { - while connects() { + while connects().await { tokio::task::yield_now().await; } }) @@ -5083,7 +5123,61 @@ mod tests { .expect("dropped guard closes readiness listener"); } + #[tokio::test] + async fn tcp_readiness_waits_for_initial_session_acceptance() { + let port = std::net::TcpListener::bind("127.0.0.1:0") + .unwrap() + .local_addr() + .unwrap() + .port(); + let (tx, rx) = tokio::sync::watch::channel(false); + let readiness = ControlReadiness::start(ReadinessEndpoint::Tcp(port), Some(rx)).unwrap(); + assert!( + !timeout( + Duration::from_millis(100), + tokio::net::TcpStream::connect(("127.0.0.1", port)) + ) + .await + .is_ok_and(|result| result.is_ok()) + ); + tx.send_replace(true); + timeout(Duration::from_secs(2), async { + while !timeout( + Duration::from_millis(100), + tokio::net::TcpStream::connect(("127.0.0.1", port)), + ) + .await + .is_ok_and(|result| result.is_ok()) + { + tokio::task::yield_now().await; + } + }) + .await + .expect("accepted session opens TCP readiness"); + drop(readiness); + } + + #[cfg(not(unix))] + #[tokio::test] + async fn unix_readiness_is_rejected_without_disabling_tcp() { + let result = ControlReadiness::start(ReadinessEndpoint::Unix("health.sock".into()), None); + assert!( + result + .err() + .expect("Unix sockets unsupported") + .to_string() + .contains("unsupported") + ); + let (_tx, rx) = tokio::sync::watch::channel(false); + assert!( + ControlReadiness::start(ReadinessEndpoint::Unix("health.sock".into()), Some(rx)) + .is_err(), + "invalid adapter must fail even before session acceptance" + ); + } + #[test] + #[cfg(unix)] fn control_readiness_rejects_relative_path() { let error = prepare_control_readiness_path(std::path::Path::new("health.sock")) .expect_err("relative readiness path must be rejected"); diff --git a/crates/openshell-supervisor/src/main.rs b/crates/openshell-supervisor/src/main.rs index 4bfb469891..505f3878bc 100644 --- a/crates/openshell-supervisor/src/main.rs +++ b/crates/openshell-supervisor/src/main.rs @@ -564,6 +564,7 @@ mod tests { #[test] fn completion_marker_must_be_absolute() { assert!(validate_main_exit_marker(Some(Path::new("relative"))).is_err()); - assert!(validate_main_exit_marker(Some(Path::new("/run/openshell/main-exit"))).is_ok()); + let absolute = std::env::temp_dir().join("openshell-main-exit"); + assert!(validate_main_exit_marker(Some(&absolute)).is_ok()); } } diff --git a/tasks/scripts/windows-msvc.ps1 b/tasks/scripts/windows-msvc.ps1 index 192c505e8f..0bc373af0f 100644 --- a/tasks/scripts/windows-msvc.ps1 +++ b/tasks/scripts/windows-msvc.ps1 @@ -50,7 +50,7 @@ if (-not [int]::TryParse($BuildJobsValue, [ref] $WindowsBuildJobs) -or $WindowsB } $WindowsCargoMutex = [System.Threading.Mutex]::new($false, "Local\OpenShellWindowsMsvcCargo") -$UnsupportedDriverPackageExcludes = "--exclude openshell-driver-docker --exclude openshell-driver-kubernetes --exclude openshell-driver-kubernetes-secrets --exclude openshell-driver-podman --exclude openshell-driver-vault --exclude openshell-driver-vm --exclude openshell-sandbox --exclude openshell-supervisor --exclude openshell-supervisor-process --exclude openshell-vfio" +$UnsupportedDriverPackageExcludes = "--exclude openshell-driver-docker --exclude openshell-driver-kubernetes --exclude openshell-driver-kubernetes-secrets --exclude openshell-driver-podman --exclude openshell-driver-vault --exclude openshell-driver-vm --exclude openshell-sandbox --exclude openshell-vfio" $WindowsClippyPackageExcludes = $UnsupportedDriverPackageExcludes $WindowsClippyLintArgs = "-D warnings -A dead-code -A unused-imports -A clippy::unused-async" $PrebuiltZ3WorkspaceFeatures = "--features openshell-prover/prebuilt-z3"