diff --git a/.agents/skills/build-openshell-mxc-windows/SKILL.md b/.agents/skills/build-openshell-mxc-windows/SKILL.md index a6f71388ff..eef3b009ea 100644 --- a/.agents/skills/build-openshell-mxc-windows/SKILL.md +++ b/.agents/skills/build-openshell-mxc-windows/SKILL.md @@ -21,6 +21,18 @@ 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. +The supervisor process-access multiplexer consumes boundary-provided streams; +do not restore local PID, PTY, or process-group operations there. Native process +mechanics belong to the sandbox or isolation backend. Keep the optional Unix +SSH access adapter separate from portable session orchestration. +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/Cargo.lock b/Cargo.lock index a36b8cd4a5..06043f6124 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5132,7 +5132,6 @@ dependencies = [ "hex", "libc", "miette", - "nix 0.29.0", "openshell-core", "openshell-isolation-interface", "openshell-ocsf", diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index 49fd66a9a8..316e590925 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -891,6 +891,47 @@ pub fn parse_env_pairs(items: &[String]) -> Result> { Ok(map) } +/// Resolve `--env-from KEY[=ENVVAR]` values from the CLI process environment. +/// +/// This keeps environment values out of process arguments while preserving the +/// same sandbox environment validation as `--env KEY=VALUE`. +pub fn parse_env_from_pairs(items: &[String]) -> Result> { + let mut map = HashMap::new(); + + for item in items { + let (key, env_name) = match item.split_once('=') { + Some((key, env_name)) => (key.trim(), env_name.trim()), + None => (item.trim(), item.trim()), + }; + if !is_valid_env_name(key) { + return Err(miette::miette!( + "--env-from key must match [A-Za-z_][A-Za-z0-9_]*; got '{key}'" + )); + } + if key.starts_with("OPENSHELL_") { + return Err(miette::miette!( + "--env-from keys starting with OPENSHELL_ are reserved; got '{key}'" + )); + } + if !is_valid_env_name(env_name) { + return Err(miette::miette!( + "--env-from source must match [A-Za-z_][A-Za-z0-9_]*; got '{env_name}'" + )); + } + if map.contains_key(key) { + return Err(miette::miette!("duplicate --env-from sandbox key '{key}'")); + } + let value = std::env::var(env_name).map_err(|_| { + miette::miette!( + "--env-from source environment variable '{env_name}' is not set or is not valid Unicode" + ) + })?; + map.insert(key.to_string(), value); + } + + Ok(map) +} + /// Resolve `--secret-material-env KEY[=ENVVAR]` values from the CLI process /// environment (`ENVVAR` defaults to `KEY`) so secrets never transit argv. pub fn parse_secret_material_env_pairs(items: &[String]) -> Result> { diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 14984388dd..f3cd9fd6e3 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1442,7 +1442,7 @@ enum SandboxCommands { name: Option, /// Create the sandbox from a named sandbox template. - #[arg(long, conflicts_with_all = ["from", "gpu", "cpu", "memory", "driver_config_json", "envs"])] + #[arg(long, conflicts_with_all = ["from", "gpu", "cpu", "memory", "driver_config_json", "envs", "env_from"])] template: Option, /// Sandbox source: a rootfs tar archive (`.tar`, `.tar.gz`, or `.tgz`) @@ -1586,6 +1586,13 @@ enum SandboxCommands { #[arg(long = "env", value_name = "KEY=VALUE")] envs: Vec, + /// Set a sandbox environment variable from the CLI process environment. + /// + /// Format: `KEY[=ENVVAR]`. When `ENVVAR` is omitted, `KEY` is used. + /// The value does not appear in the CLI process arguments. Repeatable. + #[arg(long = "env-from", value_name = "KEY[=ENVVAR]")] + env_from: Vec, + /// Suppress warnings when --env values look like credentials. #[arg(long = "no-credential-warnings")] no_credential_warnings: bool, @@ -3366,6 +3373,7 @@ async fn run_async() -> Result<()> { no_auto_providers, labels, envs, + env_from, no_credential_warnings, approval_mode, output, @@ -3403,7 +3411,12 @@ async fn run_async() -> Result<()> { } // Parse --env flags into a HashMap. - let env_map = run::parse_env_pairs(&envs)?; + let mut env_map = run::parse_env_pairs(&envs)?; + for (key, value) in run::parse_env_from_pairs(&env_from)? { + if env_map.insert(key.clone(), value).is_some() { + return Err(miette::miette!("duplicate environment key '{key}'")); + } + } // Parse --upload specs into [(local_path, sandbox_path, git_ignore)]. let upload_specs: Vec<(String, Option, bool)> = upload diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index afd5872a45..487b0eee7d 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -4,8 +4,8 @@ //! CLI command implementations. pub use crate::commands::common::{ - PolicyGetView, parse_credential_expiry_cli_value, parse_env_pairs, parse_key_value_pairs, - parse_secret_material_env_pairs, warn_credential_env_vars, + PolicyGetView, parse_credential_expiry_cli_value, parse_env_from_pairs, parse_env_pairs, + parse_key_value_pairs, parse_secret_material_env_pairs, warn_credential_env_vars, }; use crate::commands::common::{ ProvisioningDisplay, ProvisioningStep, confirm_global_setting_delete, @@ -6743,11 +6743,12 @@ mod tests { ForwardTcpConnectionError, PolicyGetView, ProvisioningStep, build_sandbox_resource_limits, format_endpoint, format_log_line, git_sync_files, has_main_process_result, parse_cli_setting_value, parse_credential_expiry_cli_value, parse_driver_config_json, - parse_secret_material_env_pairs, policy_revision_list_json, policy_revision_to_json, - proto_execution_timeout, provisioning_timeout_message, ready_false_condition_message, - relay_local_socket, resolve_from, rootfs_tar_sources_supported_for_gateway, - sandbox_should_persist, sandbox_upload_plan, service_endpoint_to_json, - service_expose_status_error, service_url_for_gateway, workspace_member_to_json, + parse_env_from_pairs, parse_secret_material_env_pairs, policy_revision_list_json, + policy_revision_to_json, proto_execution_timeout, provisioning_timeout_message, + ready_false_condition_message, relay_local_socket, resolve_from, + rootfs_tar_sources_supported_for_gateway, sandbox_should_persist, sandbox_upload_plan, + service_endpoint_to_json, service_expose_status_error, service_url_for_gateway, + workspace_member_to_json, }; use openshell_core::proto::TcpForwardFrame; @@ -7006,6 +7007,52 @@ mod tests { )); } + #[test] + fn parse_env_from_pairs_reads_named_and_same_name_environment_variables() { + let _named = EnvVarGuard::set("NAV_PARSE_ENV_FROM_NAMED", "named-value"); + let _same_name = EnvVarGuard::set("NAV_PARSE_ENV_FROM_SAME", "same-name-value"); + + let parsed = parse_env_from_pairs(&[ + "SANDBOX_NAMED=NAV_PARSE_ENV_FROM_NAMED".to_string(), + "NAV_PARSE_ENV_FROM_SAME".to_string(), + ]) + .expect("parse"); + assert_eq!( + parsed.get("SANDBOX_NAMED"), + Some(&"named-value".to_string()) + ); + assert_eq!( + parsed.get("NAV_PARSE_ENV_FROM_SAME"), + Some(&"same-name-value".to_string()) + ); + } + + #[test] + fn parse_env_from_pairs_rejects_missing_invalid_reserved_and_duplicate_keys() { + let _missing = EnvVarGuard::unset("NAV_PARSE_ENV_FROM_MISSING"); + let _present = EnvVarGuard::set("NAV_PARSE_ENV_FROM_PRESENT", "value"); + + for (input, expected) in [ + ("TARGET=NAV_PARSE_ENV_FROM_MISSING", "is not set"), + ("1BAD=NAV_PARSE_ENV_FROM_PRESENT", "key must match"), + ("TARGET=BAD-NAME", "source must match"), + ("OPENSHELL_RESERVED=NAV_PARSE_ENV_FROM_PRESENT", "reserved"), + ] { + let error = parse_env_from_pairs(&[input.to_string()]).expect_err("must reject"); + assert!( + error.to_string().contains(expected), + "unexpected error: {error}" + ); + } + + let error = parse_env_from_pairs(&[ + "TARGET=NAV_PARSE_ENV_FROM_PRESENT".to_string(), + "TARGET=NAV_PARSE_ENV_FROM_PRESENT".to_string(), + ]) + .expect_err("duplicate must reject"); + assert!(error.to_string().contains("duplicate --env-from")); + } + #[test] fn parse_secret_material_env_pairs_reads_value_from_named_environment_variable() { let _guard = EnvVarGuard::set("NAV_PARSE_SME_NAMED", "pem-material"); 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 std::process::Output { + run_cli_sandbox_create_with_xdg_and_env(server, xdg_dir, name, extra_args, &[]).await +} + +async fn run_cli_sandbox_create_with_xdg_and_env( + server: &TestServer, + xdg_dir: &TempDir, + name: &str, + extra_args: &[&str], + environment: &[(&str, &str)], ) -> std::process::Output { let mut cmd = tokio::process::Command::new(env!("CARGO_BIN_EXE_openshell")); for (key, _) in std::env::vars().filter(|(k, _)| k.starts_with("OPENSHELL_")) { @@ -3153,6 +3163,7 @@ async fn run_cli_sandbox_create_with_xdg( "--no-auto-providers", ]) .args(extra_args) + .envs(environment.iter().copied()) .env("XDG_CONFIG_HOME", xdg_dir.path()) .env("HOME", xdg_dir.path()) .env("OPENSHELL_PROVISION_TIMEOUT", "5") @@ -3263,6 +3274,47 @@ async fn sandbox_create_upload_warns_and_reaches_ssh_outside_git_repository() { ); } +async fn run_cli_sandbox_create_with_env( + server: &TestServer, + name: &str, + extra_args: &[&str], + environment: &[(&str, &str)], +) -> std::process::Output { + let xdg_dir = tempfile::tempdir().unwrap(); + prepare_cli_xdg(server, &xdg_dir); + run_cli_sandbox_create_with_xdg_and_env(server, &xdg_dir, name, extra_args, environment).await +} + +#[tokio::test] +async fn sandbox_create_env_from_reaches_request() { + let server = run_server().await; + let value = "qualification-value-not-in-argv"; + + let output = run_cli_sandbox_create_with_env( + &server, + "env-from-test", + &["--env-from", "SANDBOX_VALUE=HOST_VALUE", "--output=json"], + &[("HOST_VALUE", value)], + ) + .await; + assert!( + output.status.success(), + "sandbox create failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let requests = create_requests(&server).await; + let environment = &requests[0] + .spec + .as_ref() + .expect("spec should be present") + .environment; + assert_eq!( + environment.get("SANDBOX_VALUE").map(String::as_str), + Some(value) + ); +} + async fn run_cli_sandbox_template_create( server: &TestServer, name: &str, diff --git a/crates/openshell-core/src/paths.rs b/crates/openshell-core/src/paths.rs index 239d24b9ca..20196eaf48 100644 --- a/crates/openshell-core/src/paths.rs +++ b/crates/openshell-core/src/paths.rs @@ -57,6 +57,24 @@ pub fn openshell_state_dir() -> Result { Ok(xdg_state_dir()?.join("openshell")) } +/// Platform default for gateway audit files. Sink configuration and writing +/// remain portable; only the directory convention depends on the host. +pub fn gateway_audit_log_dir() -> PathBuf { + #[cfg(target_os = "windows")] + { + if let Some(directory) = std::env::var_os("ProgramData") { + return PathBuf::from(directory).join("OpenShell").join("logs"); + } + std::env::temp_dir().join("openshell").join("logs") + } + #[cfg(not(target_os = "windows"))] + { + openshell_state_dir() + .unwrap_or_else(|_| std::env::temp_dir().join("openshell")) + .join("logs") + } +} + /// Resolve the XDG data base directory. /// /// Returns `$XDG_DATA_HOME` if set, otherwise `$HOME/.local/share`. diff --git a/crates/openshell-core/src/provider_credentials.rs b/crates/openshell-core/src/provider_credentials.rs index 8e3a5fe850..41438e1666 100644 --- a/crates/openshell-core/src/provider_credentials.rs +++ b/crates/openshell-core/src/provider_credentials.rs @@ -399,6 +399,18 @@ impl ProviderCredentialState { .revision } + /// Whether this snapshot contains endpoint-bound material that must be + /// resolved by a network proxy rather than exposed to the child process. + #[must_use] + pub fn requires_proxy_resolution(&self) -> bool { + let inner = self + .inner + .read() + .expect("provider credential state poisoned"); + !inner.static_credential_bindings.is_empty() + || !inner.current.dynamic_credentials.is_empty() + } + /// Remove a key from the credential snapshot's child env. /// /// Used when a sandbox-side service (e.g., metadata server) fails to start diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 84c36945b3..19af1ffcc3 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -455,7 +455,7 @@ impl MxcComputeBackend { /// Test-only constructor wiring the in-process mock `wxc-exec` shim. #[cfg(test)] pub(crate) fn new_mocked(config: MxcComputeConfig) -> 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-isolation-interface/src/contract.rs b/crates/openshell-isolation-interface/src/contract.rs index ab3e10752f..246394d28e 100644 --- a/crates/openshell-isolation-interface/src/contract.rs +++ b/crates/openshell-isolation-interface/src/contract.rs @@ -857,7 +857,7 @@ pub trait BoundaryLoopbackConnector: Send + Sync { /// that owns a connection or one of its executable ancestors. A missing digest /// is `None`, never an empty value; binary-scoped policy cannot authorize an /// executable whose digest is unavailable. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ExecutableIdentity { /// Absolute executable path in the workload filesystem namespace. pub path: PathBuf, @@ -871,7 +871,7 @@ pub struct ExecutableIdentity { /// /// How a backend resolves identity is private to that backend; the shape and /// fail-closed semantics do not change. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct BinaryIdentity { /// Executable that owns the accepted connection. pub executable: ExecutableIdentity, diff --git a/crates/openshell-prover/src/containment.rs b/crates/openshell-prover/src/containment.rs index 9a9c9cb72a..ff6b8da2fb 100644 --- a/crates/openshell-prover/src/containment.rs +++ b/crates/openshell-prover/src/containment.rs @@ -2225,6 +2225,16 @@ mod tests { parse_policy_str(value).expect("valid policy") } + #[test] + fn unknown_policy_controls_are_not_silently_omitted_from_containment_checks() { + for control in ["ui", "unsupported_control"] { + let source = format!("version: 1\n{control}: {{}}\n"); + let error = + parse_policy_str(&source).expect_err("unknown policy authority must fail closed"); + assert!(error.to_string().contains(control)); + } + } + fn options() -> CheckOptions { CheckOptions::new(Duration::from_secs(10)) } diff --git a/crates/openshell-sandbox-backend/README.md b/crates/openshell-sandbox-backend/README.md index 36a3933b28..2186210cb4 100644 --- a/crates/openshell-sandbox-backend/README.md +++ b/crates/openshell-sandbox-backend/README.md @@ -24,3 +24,25 @@ the execution outcome unknown. Exec envelopes without an expiration time are rejected. Update the supervisor and sandbox runtime together when deploying this protocol change. + +## Boundary audit validation + +`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. + +## Boundary transport establishment + +Backends may supply a `BoundaryTransportConnector` for raw stream establishment. +The shared client retains TLS peer verification, session bearer authentication, +generation checks, and reconnection handling. The connector is reused for policy +discovery and attachment; it does not define platform launch configuration or +replace the isolation interface. 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..9b5b10fbd6 100644 --- a/crates/openshell-sandbox-backend/src/lib.rs +++ b/crates/openshell-sandbox-backend/src/lib.rs @@ -8,12 +8,13 @@ //! 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; pub mod sandbox_auth; -pub use runtime::OpenShellRuntimeBackend; +pub use runtime::{BoundaryTransportConnector, OpenShellRuntimeBackend}; /// Stable isolation backend name implemented by `openshell-sandbox`. pub const BACKEND_NAME: &str = "openshell-sandbox"; diff --git a/crates/openshell-sandbox-backend/src/runtime.rs b/crates/openshell-sandbox-backend/src/runtime.rs index b4a644ebb6..318e82462e 100644 --- a/crates/openshell-sandbox-backend/src/runtime.rs +++ b/crates/openshell-sandbox-backend/src/runtime.rs @@ -70,9 +70,21 @@ fn begin_recovery_window( *deadline.get_or_insert(failure_time + CONNECT_RETRY_TIMEOUT) } +/// Backend-owned raw transport establishment; shared code still authenticates +/// the resulting stream with generation-pinned TLS and Sandbox Protocol JWTs. +#[async_trait] +pub trait BoundaryTransportConnector: std::fmt::Debug + Send + Sync { + async fn connect( + &self, + descriptor: &SandboxRuntimeDescriptor, + ) -> Result; +} + /// Host-side `OpenShell` Sandbox Protocol implementation registered with the supervisor. #[derive(Debug)] pub struct OpenShellRuntimeBackend { + connector: Option>, + audit_validator: Arc, ca_file_paths: Arc>>, provider_credentials: openshell_core::provider_credentials::ProviderCredentialState, sandbox_bearer: openshell_core::jwt::SessionBearerTokenSlot, @@ -84,7 +96,16 @@ impl OpenShellRuntimeBackend { descriptor: SandboxRuntimeDescriptor, bearer: openshell_core::jwt::SessionBearerTokenSlot, ) -> Result<(Option, bool), BackendError> { - let client = BoundaryClient::new(descriptor, bearer); + Self::discover_policy_with_connector(descriptor, bearer, None).await + } + + pub async fn discover_policy_with_connector( + descriptor: SandboxRuntimeDescriptor, + bearer: openshell_core::jwt::SessionBearerTokenSlot, + connector: Option>, + ) -> Result<(Option, bool), BackendError> { + let mut client = BoundaryClient::new(descriptor, bearer); + client.connector = connector; match client.call_idempotent(Request::DiscoverPolicy).await? { Response::ImagePolicy { yaml, invalid } => Ok((yaml, invalid)), _ => Err(BackendError::Descriptor( @@ -99,11 +120,32 @@ impl OpenShellRuntimeBackend { sandbox_bearer: openshell_core::jwt::SessionBearerTokenSlot, ) -> Self { Self { + connector: None, + audit_validator: Arc::new(crate::audit::LinuxBoundaryAuditValidator), ca_file_paths, provider_credentials, sandbox_bearer, } } + + #[must_use] + pub fn with_transport_connector( + mut self, + connector: Arc, + ) -> Self { + self.connector = Some(connector); + self + } + + /// 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] @@ -127,10 +169,9 @@ impl IsolationBackend for OpenShellRuntimeBackend { let generation = runtime_descriptor.generation.clone(); let session_id = runtime_descriptor.session_id; let outer_fence = runtime_descriptor.outer_fence.clone(); - let client = Arc::new(BoundaryClient::new( - runtime_descriptor, - self.sandbox_bearer.clone(), - )); + let mut client = BoundaryClient::new(runtime_descriptor, self.sandbox_bearer.clone()); + client.connector = self.connector.clone(); + let client = Arc::new(client); let response = client .call_idempotent(Request::Attach { supervisor_instance_id: client.supervisor_instance_id, @@ -147,6 +188,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 +334,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 +375,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(); @@ -1089,6 +1125,7 @@ async fn dispatch_client_mediation_frame( } struct BoundaryClient { + connector: Option>, runtime_descriptor: SandboxRuntimeDescriptor, supervisor_instance_id: crate::boundary_protocol::SupervisorInstanceId, sandbox_bearer: openshell_core::jwt::SessionBearerTokenSlot, @@ -1252,6 +1289,7 @@ impl BoundaryClient { sandbox_bearer: openshell_core::jwt::SessionBearerTokenSlot, ) -> Self { Self { + connector: None, runtime_descriptor, supervisor_instance_id: crate::boundary_protocol::SupervisorInstanceId::new(), sandbox_bearer, @@ -1873,6 +1911,7 @@ impl BoundaryClient { &self, ) -> Result<(tonic::transport::Channel, Arc), BackendError> { let runtime_descriptor = self.runtime_descriptor.clone(); + let connector = self.connector.clone(); let transport = Arc::new(TransportAbort::default()); let connector_transport = transport.clone(); let endpoint = @@ -1886,6 +1925,7 @@ impl BoundaryClient { let channel = endpoint .connect_with_connector(tower::service_fn(move |_: tonic::transport::Uri| { let runtime_descriptor = runtime_descriptor.clone(); + let connector = connector.clone(); let abort = connector_transport.clone(); async move { // A channel is one authenticated connection. Never let it @@ -1893,7 +1933,7 @@ impl BoundaryClient { if abort.is_aborted() || abort.connected.swap(true, Ordering::AcqRel) { return Err(AbortableTransport::aborted_error()); } - connect_boundary_with_retry(&runtime_descriptor) + connect_boundary_with_retry(&runtime_descriptor, connector.clone()) .await .map(|inner| TokioIo::new(AbortableTransport { inner, abort })) .map_err(|error| std::io::Error::other(error.to_string())) @@ -1914,10 +1954,11 @@ impl BoundaryClient { async fn connect_boundary_with_retry( runtime_descriptor: &SandboxRuntimeDescriptor, + connector: Option>, ) -> Result { let deadline = tokio::time::Instant::now() + CONNECT_RETRY_TIMEOUT; loop { - match connect_boundary_once(runtime_descriptor).await { + match connect_boundary_using_connector(runtime_descriptor, connector.as_deref()).await { Ok(stream) => return Ok(stream), Err(error) if tokio::time::Instant::now() >= deadline => return Err(error), Err(_) => tokio::time::sleep(Duration::from_millis(25)).await, @@ -1925,41 +1966,53 @@ async fn connect_boundary_with_retry( } } +#[cfg(test)] async fn connect_boundary_once( runtime_descriptor: &SandboxRuntimeDescriptor, ) -> Result { - let stream: BoundaryDuplexStream = match &runtime_descriptor.transport { - #[cfg(unix)] - SandboxTransport::Unix { socket_path } => { - let stream = UnixStream::connect(socket_path).await.map_err(|error| { - BackendError::Unavailable(format!( - "connect to mapped boundary control socket {}: {error}", - socket_path.display() - )) - })?; - Box::new(stream) - } - #[cfg(not(unix))] - SandboxTransport::Unix { .. } => { - return Err(BackendError::Unavailable( - "Unix boundary transport requires a Unix host".to_string(), - )); - } - SandboxTransport::Tcp { - authority, - addresses, - } => { - let stream = openshell_core::net::connect_tcp_nodelay_best_effort(addresses) - .await - .map_err(|error| { + connect_boundary_using_connector(runtime_descriptor, None).await +} + +async fn connect_boundary_using_connector( + runtime_descriptor: &SandboxRuntimeDescriptor, + connector: Option<&dyn BoundaryTransportConnector>, +) -> Result { + let stream: BoundaryDuplexStream = if let Some(connector) = connector { + connector.connect(runtime_descriptor).await? + } else { + match &runtime_descriptor.transport { + #[cfg(unix)] + SandboxTransport::Unix { socket_path } => { + let stream = UnixStream::connect(socket_path).await.map_err(|error| { BackendError::Unavailable(format!( - "connect to boundary TLS endpoint {authority}: {error}" + "connect to mapped boundary control socket {}: {error}", + socket_path.display() )) })?; - enable_boundary_tcp_keepalive(&stream); - Box::new(stream) + Box::new(stream) + } + #[cfg(not(unix))] + SandboxTransport::Unix { .. } => { + return Err(BackendError::Unavailable( + "Unix boundary transport requires a Unix host".to_string(), + )); + } + SandboxTransport::Tcp { + authority, + addresses, + } => { + let stream = openshell_core::net::connect_tcp_nodelay_best_effort(addresses) + .await + .map_err(|error| { + BackendError::Unavailable(format!( + "connect to boundary TLS endpoint {authority}: {error}" + )) + })?; + enable_boundary_tcp_keepalive(&stream); + Box::new(stream) + } + SandboxTransport::Vsock { guest_cid, port } => connect_host_vsock(*guest_cid, *port)?, } - SandboxTransport::Vsock { guest_cid, port } => connect_host_vsock(*guest_cid, *port)?, }; let tls = &runtime_descriptor.tls; let server_name = @@ -2961,6 +3014,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, @@ -3157,6 +3211,83 @@ mod tests { server.abort(); } + #[derive(Debug)] + struct ReverseTestConnector(tokio::net::TcpListener); + + #[async_trait] + impl BoundaryTransportConnector for ReverseTestConnector { + async fn connect( + &self, + _: &SandboxRuntimeDescriptor, + ) -> Result { + let (stream, _) = self + .0 + .accept() + .await + .map_err(|error| BackendError::Unavailable(error.to_string()))?; + Ok(Box::new(stream)) + } + } + + #[tokio::test] + async fn custom_reverse_transport_preserves_tls_and_bearer_authentication() { + let certificate = test_certificate(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let connector = Arc::new(ReverseTestConnector(listener)); + let server = tokio::spawn(async move { + let stream = tokio::net::TcpStream::connect(address).await.unwrap(); + let stream = tokio_rustls::TlsAcceptor::from(certificate.server_config) + .accept(stream) + .await + .unwrap(); + serve_test_grpc(Box::new(stream), "a".repeat(32)).await; + }); + let mut client = BoundaryClient::new( + tls_runtime_descriptor(address, certificate.client_tls), + test_bearer(&"a".repeat(32)), + ); + client.connector = Some(connector); + let response = + tokio::time::timeout(Duration::from_secs(5), client.exchange(Request::Confirm)) + .await + .unwrap() + .unwrap(); + assert_eq!( + response, + Response::Confirmed { + confirmation: Box::new(test_confirmation()) + } + ); + server.abort(); + } + + #[tokio::test] + async fn custom_transport_cannot_bypass_tls_peer_verification() { + let server_certificate = test_certificate(); + let different_trust = test_certificate(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let connector = ReverseTestConnector(listener); + let server = tokio::spawn(async move { + let stream = tokio::net::TcpStream::connect(address).await.unwrap(); + let _ = tokio_rustls::TlsAcceptor::from(server_certificate.server_config) + .accept(stream) + .await; + }); + let descriptor = tls_runtime_descriptor(address, different_trust.client_tls); + assert!( + tokio::time::timeout( + Duration::from_secs(5), + connect_boundary_using_connector(&descriptor, Some(&connector)) + ) + .await + .unwrap() + .is_err() + ); + server.await.unwrap(); + } + #[tokio::test] async fn tls_tcp_rejects_tls12_only_server() { let certificate = test_certificate_with_protocol_versions(&[&rustls::version::TLS12]); diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index a8d31fbfe9..ebf2929dad 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -5,15 +5,19 @@ #[cfg(target_os = "linux")] mod accept_interrupt; +#[cfg(target_os = "linux")] pub mod boundary_exec; +#[cfg(target_os = "linux")] pub mod boundary_io; mod boundary_server; +#[cfg(target_os = "linux")] pub mod child_env; pub mod container_log; #[cfg(target_os = "linux")] pub(crate) mod delegated; #[cfg(target_os = "linux")] pub mod main_session; +#[cfg(target_os = "linux")] pub mod managed_children; #[cfg(target_os = "linux")] mod network_broker; @@ -23,7 +27,9 @@ pub mod perf; pub mod process; #[cfg(target_os = "linux")] mod provider_files; +#[cfg(target_os = "linux")] mod pty; +#[cfg(target_os = "linux")] pub mod sandbox; #[cfg(target_os = "linux")] pub mod sftp; diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 99f37b202e..53c3032389 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -24,7 +24,7 @@ use tracing_subscriber::{Layer, layer::SubscriberExt, util::SubscriberInitExt}; const COPY_SELF_SUBCOMMAND: &str = "copy-self"; const BOOTSTRAP_SUBCOMMAND: &str = "bootstrap"; const SEED_WORKSPACE_SUBCOMMAND: &str = "seed-workspace"; -#[cfg(any(target_os = "linux", test))] +#[cfg(target_os = "linux")] const KUBERNETES_BOOTSTRAP_SECRET_FILES: [&str; 3] = ["boundary.json", "tls.crt", "tls.key"]; #[cfg(target_os = "linux")] const BOOTSTRAP_INPUT_ROOT: &str = "/.openshell/bootstrap-input"; @@ -1622,7 +1622,7 @@ fn run_kubernetes_bootstrap() -> Result<()> { )) } -#[cfg(any(target_os = "linux", test))] +#[cfg(target_os = "linux")] fn stage_kubernetes_bootstrap_at(source: &Path, runtime: &Path, state: &Path) -> Result<()> { use std::fs::{self, OpenOptions}; use std::os::unix::fs::PermissionsExt as _; @@ -1679,7 +1679,7 @@ fn stage_kubernetes_bootstrap_at(source: &Path, runtime: &Path, state: &Path) -> Ok(()) } -#[cfg(any(target_os = "linux", test))] +#[cfg(target_os = "linux")] fn copy_projected_secret_file( source_root: &Path, name: &str, @@ -1697,7 +1697,7 @@ fn copy_projected_secret_file( copy_regular_file(&canonical_source, destination, mode) } -#[cfg(any(target_os = "linux", test))] +#[cfg(target_os = "linux")] fn copy_regular_file(source: &Path, destination: &Path, mode: u32) -> Result<()> { use std::fs::{self, OpenOptions}; use std::io::{Read as _, Write as _}; @@ -1734,10 +1734,12 @@ fn copy_regular_file(source: &Path, destination: &Path, mode: u32) -> Result<()> /// Seed the persistent workspace from the agent image as the final workload /// identity. This replaces the former root shell/tar init container. +#[cfg(target_os = "linux")] fn seed_kubernetes_workspace() -> Result<()> { seed_kubernetes_workspace_at(Path::new("/sandbox"), Path::new("/mnt/openshell-workspace")) } +#[cfg(target_os = "linux")] fn copy_workspace_tree(source: &Path, destination: &Path) -> Result<()> { use std::fs::{self, OpenOptions}; use std::io::{Read as _, Write as _}; @@ -1790,6 +1792,7 @@ fn copy_workspace_tree(source: &Path, destination: &Path) -> Result<()> { Ok(()) } +#[cfg(target_os = "linux")] fn seed_kubernetes_workspace_at(source: &Path, destination: &Path) -> Result<()> { use std::fs::{self, OpenOptions}; use std::io::Write as _; @@ -1834,6 +1837,13 @@ fn seed_kubernetes_workspace_at(source: &Path, destination: &Path) -> Result<()> Ok(()) } +#[cfg(not(target_os = "linux"))] +fn seed_kubernetes_workspace() -> Result<()> { + Err(miette::miette!( + "Kubernetes workspace seeding is supported only on Linux" + )) +} + #[cfg(target_os = "linux")] fn run_boundary(bootstrap: &Path, log_level: &str) -> Result<()> { let console_filter = @@ -1900,7 +1910,7 @@ fn main() -> Result<()> { run_boundary(&args.bootstrap, &args.log_level) } -#[cfg(test)] +#[cfg(all(test, target_os = "linux"))] mod tests { use super::*; use std::os::unix::fs::PermissionsExt; diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 2c84c147c3..439c285ccd 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -41,8 +41,9 @@ use openshell_core::proto::compute::v1::{ compute_driver_server::ComputeDriver, watch_sandboxes_event, }; use openshell_core::proto::{ - PlatformEvent, Sandbox, SandboxCondition, SandboxPhase, SandboxRestartPolicy, SandboxSpec, - SandboxStatus, SandboxTemplate, SandboxWorkloadTemplate, ServiceEndpoint, SshSession, + PlatformEvent, Sandbox, SandboxCondition, SandboxPhase, SandboxPolicy as ProtoSandboxPolicy, + SandboxRestartPolicy, SandboxSpec, SandboxStatus, SandboxTemplate, SandboxWorkloadTemplate, + ServiceEndpoint, SshSession, }; use openshell_core::telemetry::TelemetryComputeDriver; use openshell_core::{ObjectLabels, ObjectWorkspace}; @@ -75,6 +76,23 @@ pub type SharedComputeDriver = Arc + Send + Sync>; use provisioning_operation::ProvisioningOperationError; +/// Driver-specific values that must be delivered atomically with sandbox +/// creation without expanding the public compute-driver protobuf contract. +#[derive(Clone, Default)] +pub struct SandboxCreateRuntimeInputs { + pub effective_policy: Option, + pub launch_authentication: Option>, +} + +impl SandboxCreateRuntimeInputs { + #[must_use] + pub(crate) fn new(effective_policy: ProtoSandboxPolicy) -> Self { + Self { + effective_policy: Some(effective_policy), + launch_authentication: None, + } + } +} use traced_driver::TracedDriver; const LIFECYCLE_SWEEP_PAGE_SIZE: u32 = 1000; @@ -262,6 +280,16 @@ struct SandboxDeleteTarget { sandbox_name: String, } +/// Optional caller-owned preconditions for an identity-safe sandbox delete. +/// +/// These values are validated again while holding the sandbox lifecycle and +/// gateway-global locks, immediately before the durable `Deleting` mutation. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct SandboxDeletePreconditions { + pub expected_sandbox_id: Option, + pub expected_resource_version: Option, +} + /// Identity and driver result for a completed delete request. #[derive(Debug, Eq, PartialEq)] pub struct DeleteSandboxResult { @@ -299,6 +327,7 @@ enum BeginDelete { } #[derive(Debug, Clone)] +#[allow(clippy::struct_excessive_bools)] pub struct ComputeDriverInfoSnapshot { /// Gateway-selected driver name used for routing and `driver_config` keys. pub name: String, @@ -987,6 +1016,18 @@ impl ComputeRuntime { } pub async fn validate_sandbox_create(&self, sandbox: &Sandbox) -> Result<(), Status> { + self.validate_sandbox_create_with_runtime_inputs( + sandbox, + &SandboxCreateRuntimeInputs::default(), + ) + .await + } + + pub(crate) async fn validate_sandbox_create_with_runtime_inputs( + &self, + sandbox: &Sandbox, + runtime_inputs: &SandboxCreateRuntimeInputs, + ) -> Result<(), Status> { self.validate_caller_driver_config( sandbox .spec @@ -995,6 +1036,11 @@ impl ComputeRuntime { )?; let mut driver_sandbox = driver_sandbox_from_public(sandbox, &self.driver_info.name) .map_err(|status| *status)?; + if let Some(effective_policy) = runtime_inputs.effective_policy.as_ref() + && let Some(spec) = driver_sandbox.spec.as_mut() + { + spec.policy = Some(effective_policy.clone()); + } // Peek, never consume: create runs the same path immediately after and // must still find the token. if let Some(token) = take_staging_token(&mut driver_sandbox) { @@ -1088,11 +1134,59 @@ impl ComputeRuntime { await_main_process_attachment: bool, lifecycle_guard: SandboxLifecycleGuard, global_guard: SandboxSyncGuard, + ) -> Result { + Box::pin(self.create_sandbox_with_runtime_inputs_and_guards( + sandbox, + sandbox_token, + await_main_process_attachment, + SandboxCreateRuntimeInputs { + launch_authentication, + ..Default::default() + }, + lifecycle_guard, + global_guard, + )) + .await + } + + pub(crate) async fn create_sandbox_with_runtime_inputs( + &self, + sandbox: Sandbox, + sandbox_token: Option, + await_main_process_attachment: bool, + runtime_inputs: SandboxCreateRuntimeInputs, + ) -> Result { + let (lifecycle_guard, global_guard) = self + .sandbox_create_guards(sandbox.object_id()) + .await + .map_err(|error| { + crate::grpc::persistence_error_to_status(error, "acquire sandbox mutation lock") + })?; + Box::pin(self.create_sandbox_with_runtime_inputs_and_guards( + sandbox, + sandbox_token, + await_main_process_attachment, + runtime_inputs, + lifecycle_guard, + global_guard, + )) + .await + } + + pub(crate) async fn create_sandbox_with_runtime_inputs_and_guards( + &self, + sandbox: Sandbox, + sandbox_token: Option, + await_main_process_attachment: bool, + runtime_inputs: SandboxCreateRuntimeInputs, + lifecycle_guard: SandboxLifecycleGuard, + global_guard: SandboxSyncGuard, ) -> Result { // Defend the internal create path too, before consuming a staged archive // or persisting the sandbox. The gRPC handler checks before driver validation. self.validate_launch_signer_configured( - launch_authentication + runtime_inputs + .launch_authentication .as_ref() .is_some_and(|auth| !auth.is_empty()), )?; @@ -1116,6 +1210,11 @@ impl ComputeRuntime { let mut driver_sandbox = driver_sandbox_from_public(&sandbox, &self.driver_info.name) .map_err(|status| *status)?; + if let Some(effective_policy) = runtime_inputs.effective_policy + && let Some(spec) = driver_sandbox.spec.as_mut() + { + spec.policy = Some(effective_policy); + } if let Some(staged) = staged.as_ref() { set_rootfs_tar_path(&mut driver_sandbox, staged.path()); } @@ -1168,7 +1267,7 @@ impl ComputeRuntime { } if let Some(spec) = driver_sandbox.spec.as_mut() { spec.await_main_process_attachment = await_main_process_attachment; - spec.launch_authentication = launch_authentication.unwrap_or_default(); + spec.launch_authentication = runtime_inputs.launch_authentication.unwrap_or_default(); } let result = Box::pin(self.await_provisioning_operation( &sandbox, @@ -2282,8 +2381,12 @@ impl ComputeRuntime { workspace: &str, name: &str, ) -> Result { - self.delete_sandbox_allow_missing(workspace, name, false) - .await + self.delete_sandbox_with_preconditions( + workspace, + name, + SandboxDeletePreconditions::default(), + ) + .await } pub(crate) async fn delete_sandbox_allow_missing( @@ -2292,6 +2395,55 @@ impl ComputeRuntime { name: &str, allow_missing: bool, ) -> Result { + self.delete_sandbox_with_options( + workspace, + name, + allow_missing, + SandboxDeletePreconditions::default(), + ) + .await + } + + pub(crate) async fn delete_sandbox_with_preconditions( + &self, + workspace: &str, + name: &str, + preconditions: SandboxDeletePreconditions, + ) -> Result { + self.delete_sandbox_with_options(workspace, name, false, preconditions) + .await + } + + pub(crate) async fn delete_sandbox_allow_missing_with_preconditions( + &self, + workspace: &str, + name: &str, + allow_missing: bool, + preconditions: SandboxDeletePreconditions, + ) -> Result { + if preconditions == SandboxDeletePreconditions::default() { + return self + .delete_sandbox_allow_missing(workspace, name, allow_missing) + .await; + } + self.delete_sandbox_with_options(workspace, name, allow_missing, preconditions) + .await + } + + async fn delete_sandbox_with_options( + &self, + workspace: &str, + name: &str, + allow_missing: bool, + preconditions: SandboxDeletePreconditions, + ) -> Result { + if preconditions.expected_resource_version.is_some() + && preconditions.expected_sandbox_id.is_none() + { + return Err(Status::invalid_argument( + "expected_resource_version requires expected_sandbox_id", + )); + } // Resolve and acquire both request-side locks before spawning the // owned worker. Cancellation while any of these awaits is pending is // harmless because no mutation or detached work has started. @@ -2309,11 +2461,20 @@ impl ComputeRuntime { } return Err(Status::not_found("sandbox not found")); }; + if preconditions + .expected_sandbox_id + .as_deref() + .is_some_and(|expected| expected != candidate.object_id()) + { + return Err(Status::aborted( + "sandbox identity does not match expected_sandbox_id", + )); + } let target = SandboxDeleteTarget { sandbox_id: candidate.object_id().to_string(), sandbox_name: candidate.object_name().to_string(), }; - self.delete_sandbox_target(target).await + self.delete_sandbox_target(target, preconditions).await } pub(crate) async fn delete_sandbox_by_id( @@ -2321,16 +2482,20 @@ impl ComputeRuntime { sandbox_id: &str, sandbox_name: &str, ) -> Result { - self.delete_sandbox_target(SandboxDeleteTarget { - sandbox_id: sandbox_id.to_string(), - sandbox_name: sandbox_name.to_string(), - }) + self.delete_sandbox_target( + SandboxDeleteTarget { + sandbox_id: sandbox_id.to_string(), + sandbox_name: sandbox_name.to_string(), + }, + SandboxDeletePreconditions::default(), + ) .await } async fn delete_sandbox_target( &self, target: SandboxDeleteTarget, + preconditions: SandboxDeletePreconditions, ) -> Result { let delete_guard = self.lifecycle_gates.lock_for(&target.sandbox_id).await; let global_guard = self.lock_global_for_lifecycle(&delete_guard).await; @@ -2344,7 +2509,13 @@ impl ComputeRuntime { let request_span = tracing::Span::current(); tokio::spawn( async move { - Box::pin(runtime.delete_sandbox_inner(target, delete_guard, global_guard)).await + Box::pin(runtime.delete_sandbox_inner( + target, + preconditions, + delete_guard, + global_guard, + )) + .await } .instrument(request_span), ) @@ -2359,6 +2530,7 @@ impl ComputeRuntime { async fn delete_sandbox_inner( &self, target: SandboxDeleteTarget, + preconditions: SandboxDeletePreconditions, delete_guard: SandboxLifecycleGuard, guard: tokio::sync::OwnedMutexGuard<()>, ) -> Result { @@ -2382,6 +2554,23 @@ impl ComputeRuntime { "sandbox name changed while the delete request was waiting; retry explicitly", )); } + if preconditions + .expected_sandbox_id + .as_deref() + .is_some_and(|expected| expected != current.object_id()) + { + return Err(Status::aborted( + "sandbox identity changed before delete mutation", + )); + } + if preconditions + .expected_resource_version + .is_some_and(|expected| expected != sandbox_resource_version(¤t)) + { + return Err(Status::aborted( + "sandbox resource version changed before delete mutation", + )); + } // `Started` carries both sides of the CAS transition: the durable // `Deleting` row used to fence recovery, and the prior row used only @@ -7230,6 +7419,7 @@ pub fn new_test_runtime_with_driver( mod tests { use super::*; use futures::stream; + use openshell_core::proto::SandboxPolicy as PublicSandboxPolicy; use openshell_core::proto::compute::v1::{ CreateSandboxResponse, DeleteSandboxResponse, GetCapabilitiesResponse, GetSandboxRequest, GetSandboxResponse, StartSandboxResponse, StopSandboxRequest, StopSandboxResponse, @@ -7592,7 +7782,9 @@ mod tests { workspace_rpcs_unimplemented: bool, omit_protocol_metadata: bool, requires_launch_authentication: bool, + validate_create_calls: AtomicUsize, create_calls: AtomicUsize, + created_sandboxes: TestMutex>, } #[tonic::async_trait] @@ -7650,6 +7842,7 @@ mod tests { &self, _request: Request, ) -> Result, Status> { + self.validate_create_calls.fetch_add(1, Ordering::Relaxed); Ok(tonic::Response::new(ValidateSandboxCreateResponse {})) } @@ -7699,9 +7892,15 @@ mod tests { async fn create_sandbox( &self, - _request: Request, + request: Request, ) -> Result, Status> { self.create_calls.fetch_add(1, Ordering::SeqCst); + if let Some(sandbox) = request.into_inner().sandbox { + self.created_sandboxes + .lock() + .expect("created sandbox observation lock poisoned") + .push(sandbox); + } Ok(tonic::Response::new(CreateSandboxResponse::default())) } @@ -8316,6 +8515,81 @@ mod tests { } } + #[tokio::test] + async fn create_runtime_inputs_reach_driver_without_persisting_effective_policy_or_secrets() { + let driver = Arc::new(TestDriver::default()); + let runtime = test_runtime(driver.clone()).await; + + let mut sandbox = sandbox_record( + "sb-provider-inputs", + "provider-inputs", + SandboxPhase::Provisioning, + ); + sandbox.spec = Some(SandboxSpec { + policy: Some(PublicSandboxPolicy { + version: 1, + ..Default::default() + }), + ..Default::default() + }); + let mut runtime_inputs = SandboxCreateRuntimeInputs::new(PublicSandboxPolicy { + version: 2, + ..Default::default() + }); + let launch_authentication = b"protected-launch-material".to_vec(); + runtime_inputs.launch_authentication = Some(launch_authentication.clone()); + + runtime + .create_sandbox_with_runtime_inputs(sandbox, None, false, runtime_inputs) + .await + .expect("create should succeed"); + + let observed_policy_version = { + let created = driver + .created_sandboxes + .lock() + .expect("created sandbox observation lock poisoned"); + assert_eq!( + created[0].spec.as_ref().unwrap().launch_authentication, + launch_authentication, + "launch authentication must reach only the driver" + ); + created[0] + .spec + .as_ref() + .and_then(|spec| spec.policy.as_ref()) + .map(|policy| policy.version) + }; + assert_eq!( + observed_policy_version, + Some(2), + "the driver must receive the effective policy" + ); + + let persisted = runtime + .store + .get_message::("sb-provider-inputs") + .await + .expect("persisted sandbox lookup should succeed") + .expect("sandbox should be persisted"); + assert!( + !persisted + .encode_to_vec() + .windows(launch_authentication.len()) + .any(|bytes| bytes == launch_authentication), + "launch authentication must not enter the public sandbox record" + ); + assert_eq!( + persisted + .spec + .as_ref() + .and_then(|spec| spec.policy.as_ref()) + .map(|policy| policy.version), + Some(1), + "the public sandbox must retain its base policy" + ); + } + async fn test_runtime_with_gateway_managed_lifecycle( driver: SharedComputeDriver, driver_name: &str, @@ -10278,7 +10552,7 @@ mod tests { r#type: "Ready".to_string(), status: "True".to_string(), reason: "AgentRunning".to_string(), - message: "MXC workload is running".to_string(), + message: "Workload is running".to_string(), transition_time: None, }); @@ -12465,6 +12739,133 @@ mod tests { ); } + #[tokio::test] + async fn identity_guarded_delete_rejects_a_different_sandbox_before_mutation() { + let driver = ControlledDriver::new(); + let runtime = test_runtime(driver.clone()).await; + let sandbox = sandbox_record("sb-current", "sandbox-a", SandboxPhase::Ready); + runtime.store.put_message(&sandbox).await.unwrap(); + + let error = runtime + .delete_sandbox_with_preconditions( + "default", + "sandbox-a", + SandboxDeletePreconditions { + expected_sandbox_id: Some("sb-stale".to_string()), + expected_resource_version: None, + }, + ) + .await + .unwrap_err(); + + assert_eq!(error.code(), Code::Aborted); + assert_eq!(driver.delete_calls(), 0); + let current = runtime + .store + .get_message::("sb-current") + .await + .unwrap() + .unwrap(); + assert_eq!( + SandboxPhase::try_from(current.phase()).unwrap(), + SandboxPhase::Ready + ); + } + + #[tokio::test] + async fn identity_guarded_delete_revalidates_resource_version_under_lifecycle_lock() { + let driver = ControlledDriver::new(); + let runtime = test_runtime(driver.clone()).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Ready); + runtime.store.put_message(&sandbox).await.unwrap(); + let current = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .unwrap(); + let expected_resource_version = sandbox_resource_version(¤t); + + let delete_gate = runtime.lifecycle_gates.gate_for(sandbox.object_id()); + let delete_guard = delete_gate.lock().await; + let delete_runtime = runtime.clone(); + let delete = tokio::spawn(async move { + delete_runtime + .delete_sandbox_with_preconditions( + "default", + "sandbox-a", + SandboxDeletePreconditions { + expected_sandbox_id: Some("sb-1".to_string()), + expected_resource_version: Some(expected_resource_version), + }, + ) + .await + }); + tokio::time::timeout(Duration::from_secs(1), async { + while Arc::strong_count(&delete_gate) < 2 { + tokio::task::yield_now().await; + } + }) + .await + .expect("guarded delete did not start waiting on the sandbox gate"); + + runtime + .store + .update_message_cas::( + sandbox.object_id(), + expected_resource_version, + |sandbox| sandbox.set_current_policy_version(9), + ) + .await + .unwrap(); + drop(delete_guard); + + let error = delete.await.unwrap().unwrap_err(); + assert_eq!(error.code(), Code::Aborted); + assert_eq!(driver.delete_calls(), 0); + let current = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .unwrap(); + assert_eq!(current.current_policy_version(), 9); + assert_eq!( + SandboxPhase::try_from(current.phase()).unwrap(), + SandboxPhase::Ready + ); + } + + #[tokio::test] + async fn identity_guarded_delete_accepts_the_exact_current_identity() { + let driver = ControlledDriver::new(); + let runtime = test_runtime(driver.clone()).await; + let sandbox = sandbox_record("sb-1", "sandbox-a", SandboxPhase::Ready); + runtime.store.put_message(&sandbox).await.unwrap(); + let current = runtime + .store + .get_message::(sandbox.object_id()) + .await + .unwrap() + .unwrap(); + + let result = runtime + .delete_sandbox_with_preconditions( + "default", + "sandbox-a", + SandboxDeletePreconditions { + expected_sandbox_id: Some(current.object_id().to_string()), + expected_resource_version: Some(sandbox_resource_version(¤t)), + }, + ) + .await + .unwrap(); + + assert!(result.acknowledged()); + assert_eq!(result.sandbox_id, "sb-1"); + assert_eq!(driver.delete_calls(), 1); + } + #[tokio::test] async fn request_cancellation_does_not_cancel_the_delete_worker() { let driver = ControlledDriver::new(); diff --git a/crates/openshell-server/src/credentials.rs b/crates/openshell-server/src/credentials.rs index 901a44f9f3..148acaa176 100644 --- a/crates/openshell-server/src/credentials.rs +++ b/crates/openshell-server/src/credentials.rs @@ -110,6 +110,10 @@ pub trait CredentialDriver: std::fmt::Debug + Send + Sync { pub struct ResolvedProviderCredentials { pub values: HashMap, pub expires_at_ms: HashMap, + /// Keys returned by a credential driver whose effective expiration has + /// already passed. Values stay withheld, but create-time consumers need + /// the identities to fail closed instead of silently omitting credentials. + pub expired_keys: HashSet, } #[derive(Debug, Clone, Copy)] @@ -735,6 +739,7 @@ impl CredentialRuntime { effective_expires_at_ms, "skipping expired handle-backed credential" ); + resolved.expired_keys.insert(credential_key); continue; } resolved diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index bf115370b4..2bf0f93323 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -3501,21 +3501,61 @@ pub(super) async fn handle_get_gateway_config( })) } +/// Resolve the effective create-time policy without exporting credential state. +/// Provider refresh and secret delivery remain in the supervisor session. +pub(super) async fn resolve_sandbox_create_runtime_inputs( + state: &ServerState, + sandbox: &Sandbox, +) -> Result { + let sandbox_id = sandbox.object_id(); + let workspace = sandbox.object_workspace(); + let provider_names = sandbox + .spec + .as_ref() + .map(|spec| spec.providers.as_slice()) + .unwrap_or_default(); + let provider_profile_catalog = state + .provider_profile_sources + .snapshot_catalog(state.store.as_ref(), workspace) + .await?; + let provider_records = super::provider::load_provider_environment_records( + state.store.as_ref(), + workspace, + provider_names, + ) + .await?; + let effective_policy = current_effective_policy_from_records( + state, + &provider_profile_catalog, + sandbox, + sandbox_id, + &provider_records, + ) + .await?; + let policy_credential_bindings = + policy_static_credential_endpoint_bindings(Some(&effective_policy))?; + validate_policy_credential_binding_context( + &provider_profile_catalog, + &provider_records, + &effective_policy, + &policy_credential_bindings, + )?; + Ok(crate::compute::SandboxCreateRuntimeInputs::new( + effective_policy, + )) +} + pub(super) async fn handle_get_sandbox_provider_environment( state: &Arc, request: Request, ) -> Result, Status> { let sandbox_id = request.get_ref().sandbox_id.clone(); let supports_static_credential_bindings = request.get_ref().supports_static_credential_bindings; - crate::auth::guard::enforce_sandbox_scope(&request, &sandbox_id)?; + let principal = crate::auth::guard::enforce_sandbox_scope(&request, &sandbox_id)?; drop(request); - let sandbox = state - .store - .get_message::(&sandbox_id) - .await - .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? - .ok_or_else(|| Status::not_found("sandbox not found"))?; + let sandbox = + super::sandbox::fetch_and_authorize_sandbox(state, &principal, &sandbox_id).await?; let environment = load_sandbox_provider_environment(state, &sandbox, supports_static_credential_bindings) .await?; @@ -7006,17 +7046,17 @@ async fn sandbox_policy_merge_validation_data_with_catalog( ) -> Result { let global_settings = load_global_settings(state.store.as_ref()).await?; let composition_enabled = provider_policy_composition_enabled_in(&global_settings)?; - let ProviderPolicyContext { - layers, - credentialed_scopes, - endpointless_provider_names, - } = provider_policy_context_with_catalog( + let records = super::provider::load_provider_environment_records( state.store.as_ref(), - catalog, workspace, provider_names, ) .await?; + let ProviderPolicyContext { + layers, + credentialed_scopes, + endpointless_provider_names, + } = provider_policy_context_from_records(catalog, &records); let provider_layers = if composition_enabled { layers } else { @@ -7027,12 +7067,6 @@ async fn sandbox_policy_merge_validation_data_with_catalog( provider_layer_count = provider_layers.len(), "Composed provider policy and credential context for merge validation" ); - let records = super::provider::load_provider_environment_records( - state.store.as_ref(), - workspace, - provider_names, - ) - .await?; Ok(SandboxPolicyMergeValidationData { provider_layers, catalog: catalog.clone(), @@ -8476,6 +8510,18 @@ mod tests { .expect("test global policy must be present") } + #[tokio::test] + async fn create_runtime_inputs_preserve_base_policy_without_credential_sink() { + let state = test_server_state().await; + let policy = openshell_policy::restrictive_default_policy(); + let sandbox = test_sandbox("create-policy", "create-policy", policy.clone(), Vec::new()); + let inputs = resolve_sandbox_create_runtime_inputs(state.as_ref(), &sandbox) + .await + .expect("driver-independent effective policy"); + assert_eq!(inputs.effective_policy, Some(policy)); + assert!(inputs.launch_authentication.is_none()); + } + #[tokio::test] async fn list_sandbox_policies_traverses_multiple_pages_exactly_once() { let state = test_server_state().await; @@ -23625,6 +23671,22 @@ mod tests { "handle_get_sandbox_config must return NotFound, not PermissionDenied" ); + // --- handle_get_sandbox_provider_environment --- + let err = handle_get_sandbox_provider_environment( + &state, + non_member_request(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sandbox-other".into(), + supports_static_credential_bindings: true, + }), + ) + .await + .unwrap_err(); + assert_eq!( + err.code(), + Code::NotFound, + "handle_get_sandbox_provider_environment must hide cross-workspace sandboxes" + ); + // --- handle_get_sandbox_logs --- let err = handle_get_sandbox_logs( &state, diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index cc2395fe7d..903127b95a 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -75,6 +75,11 @@ pub(super) struct ProviderEnvironment { pub static_credential_bindings: HashMap, pub static_credential_keys: HashSet, pub files: HashMap, + /// Static credential keys withheld because they were already expired at + /// resolution time. Excluded from `environment`/`static_credential_keys` + /// like any other withheld key, but tracked separately from keys that + /// never had an injectable credential. + pub expired_static_keys: HashSet, } /// Immutable provider records used to build one provider-environment response. @@ -1131,6 +1136,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin let mut files = HashMap::new(); let mut file_env_keys = HashSet::new(); let mut readiness_reason = openshell_core::proto::ProviderReadinessReason::Unspecified; + let mut expired_static_keys = HashSet::new(); let now_ms = crate::persistence::current_time_ms(); validate_provider_environment_records_unique_at(store, catalog, records, now_ms).await?; let registry = openshell_providers::ProviderRegistry::new(); @@ -1257,6 +1263,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin ); readiness_reason = openshell_core::proto::ProviderReadinessReason::CredentialExpired; + expired_static_keys.insert(key.clone()); continue; } expires.entry(key.clone()).or_insert(expires_at_ms); @@ -1367,6 +1374,22 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin } } + // The credential runtime withholds expired handle-backed values. Keep + // the identity of otherwise injectable keys in the resolver result. + for key in resolved_refs.expired_keys { + if accepted_stored_credential_keys + .as_ref() + .is_some_and(|accepted| !accepted.contains(&key)) + || is_non_injectable_provider_credential(provider, &key) + || broker_only_credential_keys.contains(&key) + || has_no_usable_endpoint + || !is_valid_env_key(&key) + { + continue; + } + expired_static_keys.insert(key); + } + // Build each provider's emitted environment independently so another // provider's earlier output cannot change how this provider classifies // or populates its own keys. Cross-provider credential/config @@ -1433,6 +1456,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin static_credential_bindings, static_credential_keys, files, + expired_static_keys, }) } @@ -11858,6 +11882,49 @@ mod tests { ); } + #[tokio::test] + async fn resolve_provider_env_preserves_expired_handle_key_for_create_time_rejection() { + let store = test_store().await; + let config = openshell_core::Config::new(None).with_credential_drivers(["test-static"]); + let credentials = crate::credentials::CredentialRuntime::from_config(&config).unwrap(); + let catalog = ProviderProfileSources::with_default_sources() + .snapshot_catalog(&store, "default") + .await + .unwrap(); + let mut provider = provider_with_credential_value( + "github-expired", + "github", + "GITHUB_TOKEN", + "github-token", + ); + provider.credential_expiration_times.insert( + "GITHUB_TOKEN".to_string(), + ts(crate::persistence::current_time_ms() - 1), + ); + create_provider_record_validating( + &store, + "default", + &catalog, + provider, + Some(&credentials), + ) + .await + .unwrap(); + + let result = resolve_provider_environment_with_credentials( + &store, + &catalog, + "default", + &["github-expired".to_string()], + &credentials, + ) + .await + .unwrap(); + + assert!(!result.contains_key("GITHUB_TOKEN")); + assert!(result.expired_static_keys.contains("GITHUB_TOKEN")); + } + #[tokio::test] async fn resolve_provider_env_skips_expired_credentials_and_returns_expiry_metadata() { let store = test_store().await; diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 8f2bf9a784..a88ee85d73 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -14,6 +14,7 @@ use crate::auth::workspace_authz::{ AuthorizedWorkspaceScope, MinWorkspaceRole, authorize_list_workspace_selector, authorize_sandbox_workspace, authorize_workspace, }; +use crate::compute::SandboxDeletePreconditions; use crate::pagination::Pagination; use crate::persistence::{ ObjectLabels, ObjectListQuery, ObjectType, WriteCondition, generate_name, @@ -158,6 +159,37 @@ impl Drop for WatchSandboxStream { } } +/// Fetch a runtime sandbox ID and authorize its persisted workspace without +/// revealing whether an inaccessible object exists. +pub(super) async fn fetch_and_authorize_sandbox( + state: &ServerState, + principal: &crate::auth::principal::Principal, + sandbox_id: &str, +) -> Result { + let sandbox = state + .store + .get_message::(sandbox_id) + .await + .map_err(|error| Status::internal(format!("fetch sandbox failed: {error}")))? + .ok_or_else(|| Status::not_found("sandbox not found"))?; + authorize_sandbox_workspace( + &state.store, + &state.admin_role, + principal, + sandbox.object_workspace(), + MinWorkspaceRole::User, + ) + .await + .map_err(|error| { + if error.code() == tonic::Code::PermissionDenied { + Status::not_found("sandbox not found") + } else { + error + } + })?; + Ok(sandbox) +} + /// Resolve a public sandbox name and authorize its persisted workspace. /// Missing and unauthorized objects deliberately share one response so names /// cannot be used as an existence oracle. @@ -634,12 +666,15 @@ async fn handle_create_sandbox_inner( ) .await?; + let mut runtime_inputs = + super::policy::resolve_sandbox_create_runtime_inputs(state.as_ref(), &sandbox).await?; + state .compute .validate_launch_signer_configured(state.sandbox_session_jwt_authority.is_some())?; state .compute - .validate_sandbox_create(&sandbox) + .validate_sandbox_create_with_runtime_inputs(&sandbox, &runtime_inputs) .await .map_err(|status| { warn!(error = %status, "Rejecting sandbox create request"); @@ -669,12 +704,13 @@ async fn handle_create_sandbox_inner( .map_err(|error| Status::internal(format!("encode launch authentication: {error}"))) }) .transpose()?; + runtime_inputs.launch_authentication = launch_authentication; - let sandbox = Box::pin(state.compute.create_sandbox_authenticated_with_guards( + let sandbox = Box::pin(state.compute.create_sandbox_with_runtime_inputs_and_guards( sandbox, sandbox_token, - launch_authentication, await_main_process_attachment, + runtime_inputs, sandbox_lifecycle_guard, sandbox_sync_guard, )) @@ -1642,10 +1678,18 @@ async fn handle_delete_sandbox_inner( let workspace = super::workspace::resolve_workspace(state.store.as_ref(), &authz.workspace) .await? .name; - + // Main's canonical public delete request contains no caller-provided + // identity/version fields. The compute layer still binds the resolved + // object ID and lifecycle guards before committing deletion. + let preconditions = SandboxDeletePreconditions::default(); let result = state .compute - .delete_sandbox_allow_missing(&workspace, &name, req.allow_missing) + .delete_sandbox_allow_missing_with_preconditions( + &workspace, + &name, + req.allow_missing, + preconditions, + ) .await?; if !result.sandbox_id.is_empty() { state.telemetry.end_sandbox_session(&result.sandbox_id); @@ -2600,8 +2644,8 @@ pub(super) async fn handle_forward_tcp( .await .map_err(|e| Status::unavailable(format!("supervisor relay failed: {e}")))?; - let sandbox_id = sandbox.object_id().to_string(); let (tx, rx) = mpsc::channel::>(256); + let sandbox_id = sandbox.object_id().to_string(); tokio::spawn(async move { let _connection_guard = connection_guard; let Some(relay_stream) = @@ -2784,13 +2828,15 @@ fn validate_tcp_target_parts(host: &str, _port: u32) -> Result { } } -async fn bridge_forward_tcp_stream( +async fn bridge_forward_tcp_stream( mut inbound: tonic::Streaming, - relay_stream: tokio::io::DuplexStream, + relay_stream: S, tx: mpsc::Sender>, sandbox_id: &str, channel_id: &str, -) { +) where + S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static, +{ let (mut relay_read, mut relay_write) = tokio::io::split(relay_stream); let sandbox_id_in = sandbox_id.to_string(); @@ -5622,6 +5668,68 @@ mod tests { ); } + #[tokio::test] + async fn internal_delete_rejects_expected_identity_drift_before_mutation() { + let state = test_server_state().await; + let mut sandbox = test_sandbox("guarded-delete", Vec::new()); + sandbox.metadata.as_mut().unwrap().id = "sb-current".to_string(); + state.store.put_message(&sandbox).await.unwrap(); + + let error = state + .compute + .delete_sandbox_with_preconditions( + "default", + "guarded-delete", + SandboxDeletePreconditions { + expected_sandbox_id: Some("sb-stale".to_string()), + expected_resource_version: None, + }, + ) + .await + .unwrap_err(); + + assert_eq!(error.code(), tonic::Code::Aborted); + assert!( + state + .store + .get_message::("sb-current") + .await + .unwrap() + .is_some() + ); + } + + #[tokio::test] + async fn internal_delete_rejects_resource_version_without_immutable_identity() { + let state = test_server_state().await; + let mut sandbox = test_sandbox("guarded-delete", Vec::new()); + sandbox.metadata.as_mut().unwrap().id = "sb-current".to_string(); + state.store.put_message(&sandbox).await.unwrap(); + + let error = state + .compute + .delete_sandbox_with_preconditions( + "default", + "guarded-delete", + SandboxDeletePreconditions { + expected_sandbox_id: None, + expected_resource_version: Some(17), + }, + ) + .await + .unwrap_err(); + + assert_eq!(error.code(), tonic::Code::InvalidArgument); + assert!( + state + .store + .get_message::("sb-current") + .await + .unwrap() + .is_some() + ); + } + #[tokio::test] async fn attach_sandbox_provider_persists_current_provider_list() { let state = test_server_state().await; diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 27dda635d8..8df96a7898 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -1208,6 +1208,19 @@ pub enum ComputeDriverInstance { /// Factory for a compute driver linked into a gateway binary. #[async_trait::async_trait] pub trait ComputeDriverFactory: Send + Sync { + /// Resolve the driver's external-resource admission contract. Defaults to + /// the operator-configured policy, including secure admission defaults. + /// Drivers without resource-label support may override this explicitly. + fn admission_policy( + &self, + context: ComputeDriverConfigContext<'_>, + ) -> Result { + compute::driver_config::admission_config_from_context( + context.driver_startup, + context.driver_name, + ) + } + /// Validate selected-driver configuration without starting a driver, /// connecting a transport, or modifying runtime state. /// @@ -1609,8 +1622,20 @@ async fn build_compute_runtime( false, )?; let telemetry_compute_driver = driver.telemetry_compute_driver(registry); - let admission = - compute::driver_config::admission_config_from_context(driver_startup, driver.name())?; + let admission = match &driver { + ConfiguredComputeDriver::Registered(registration) => registration + .factory + .admission_policy(ComputeDriverConfigContext { + driver_name: ®istration.name, + gateway_name: &config.name, + gateway_bind_address: config.bind_address, + gateway_log_level: &config.log_level, + driver_startup, + })?, + ConfiguredComputeDriver::Remote { name } => { + compute::driver_config::admission_config_from_context(driver_startup, name)? + } + }; info!(driver = %driver.name(), "Using compute driver"); let runtime = match driver { ConfiguredComputeDriver::Registered(registration) => { @@ -2078,6 +2103,29 @@ mod tests { #[derive(Clone, Copy)] struct TestComputeDriverFactory; + #[test] + fn factory_admission_defaults_remain_enabled() { + use super::ComputeDriverFactory as _; + let endpoint_overrides = std::collections::BTreeMap::new(); + let context = super::ComputeDriverConfigContext { + driver_name: "custom", + gateway_name: "test", + gateway_bind_address: "127.0.0.1:8080".parse().unwrap(), + gateway_log_level: "info", + driver_startup: crate::compute::driver_config::DriverStartupContext { + file: None, + guest_tls: None, + gateway_port: 8080, + gateway_tls_enabled: false, + endpoint_overrides: &endpoint_overrides, + }, + }; + let policy = TestComputeDriverFactory.admission_policy(context).unwrap(); + assert!(policy.resource_admission.enabled); + assert_eq!(policy.resource_admission.required_labels.len(), 2); + assert!(!policy.allow_driver_config); + } + // Omitting validate_config exercises source compatibility for out-of-tree // factories written before package preflight introduced that hook. #[async_trait::async_trait] diff --git a/crates/openshell-server/src/tracing_setup.rs b/crates/openshell-server/src/tracing_setup.rs index 3a74edd72f..589c4f1aba 100644 --- a/crates/openshell-server/src/tracing_setup.rs +++ b/crates/openshell-server/src/tracing_setup.rs @@ -93,7 +93,7 @@ pub fn install( ) }, ); - let (jsonl_layer, jsonl_dir) = build_ocsf_jsonl_layer(gateway.compute_driver()); + let (jsonl_layer, jsonl_dir) = build_ocsf_jsonl_layer(); // Keep the audit sink independent from the operator's diagnostic log // level. An explicit JSONL opt-in must keep every OCSF event even when the @@ -161,8 +161,7 @@ pub fn install( /// Build the OCSF JSONL audit layer for the gateway, plus the directory it /// writes into (for a one-line startup log). Returns `(None, None)` when -/// the target is not Windows, the selected compute driver is not MXC, the sink -/// was not explicitly enabled through `OPENSHELL_OCSF_JSON`, or the target +/// the sink was not explicitly enabled through `OPENSHELL_OCSF_JSON`, or the target /// directory/appender cannot be opened. /// /// The appender is *synchronous* (not wrapped in `tracing_appender::non_blocking`) @@ -171,31 +170,26 @@ pub fn install( /// its non-blocking guard on graceful shutdown), the gateway's ETW capture path /// can be force-killed by the harness, and we do not want to lose the tail of the /// audit trail. -#[cfg(not(target_os = "windows"))] -fn build_ocsf_jsonl_layer( - _compute_driver: Option<&str>, -) -> ( +fn build_ocsf_jsonl_layer() -> ( Option>, Option, ) { - // The gateway-local JSONL sink belongs to the Windows/MXC ETW path. A - // cross-platform sink needs an explicit storage and configuration contract. - (None, None) + let requested = std::env::var("OPENSHELL_OCSF_JSON").ok(); + if !ocsf_jsonl_requested(requested.as_deref()) { + return (None, None); + } + + open_ocsf_jsonl_layer(ocsf_log_dir()) } -#[cfg(target_os = "windows")] -fn build_ocsf_jsonl_layer( - compute_driver: Option<&str>, +/// Portable sink construction; environment and platform defaults are resolved +/// separately so operators and tests can supply an explicit directory. +fn open_ocsf_jsonl_layer( + dir: std::path::PathBuf, ) -> ( Option>, Option, ) { - let requested = std::env::var("OPENSHELL_OCSF_JSON").ok(); - if !mxc_ocsf_jsonl_requested(compute_driver, requested.as_deref()) { - return (None, None); - } - - let dir = ocsf_log_dir(); if let Err(e) = std::fs::create_dir_all(&dir) { eprintln!( "openshell: could not create OCSF JSONL log dir {}: {e}", @@ -222,24 +216,21 @@ fn build_ocsf_jsonl_layer( } } -/// Whether this gateway explicitly requested the Windows/MXC JSONL sink. +/// Whether this gateway explicitly requested the JSONL sink. /// Unknown values fail closed so a typo cannot unexpectedly retain audit data. -#[cfg(any(target_os = "windows", test))] -fn mxc_ocsf_jsonl_requested(compute_driver: Option<&str>, value: Option<&str>) -> bool { - compute_driver == Some("mxc") - && value.is_some_and(|value| { - matches!( - value.trim().to_ascii_lowercase().as_str(), - "1" | "true" | "on" | "yes" - ) - }) +fn ocsf_jsonl_requested(value: Option<&str>) -> bool { + value.is_some_and(|value| { + matches!( + value.trim().to_ascii_lowercase().as_str(), + "1" | "true" | "on" | "yes" + ) + }) } /// Resolve the directory for the OCSF JSONL audit file. /// /// Precedence: `OPENSHELL_OCSF_LOG_DIR` (harness / operator override) then -/// `%PROGRAMDATA%\OpenShell\logs`. -#[cfg(target_os = "windows")] +/// the platform's gateway audit directory. fn ocsf_log_dir() -> std::path::PathBuf { if let Ok(dir) = std::env::var("OPENSHELL_OCSF_LOG_DIR") { let trimmed = dir.trim(); @@ -247,10 +238,7 @@ fn ocsf_log_dir() -> std::path::PathBuf { return std::path::PathBuf::from(trimmed); } } - if let Ok(pd) = std::env::var("ProgramData") { - return std::path::PathBuf::from(pd).join("OpenShell").join("logs"); - } - std::env::temp_dir().join("openshell").join("logs") + openshell_core::paths::gateway_audit_log_dir() } #[cfg(test)] @@ -264,7 +252,45 @@ mod tests { use tracing_subscriber::EnvFilter; use tracing_subscriber::prelude::*; - use super::mxc_ocsf_jsonl_requested; + use super::ocsf_jsonl_requested; + + #[test] + fn portable_gateway_audit_sink_writes_jsonl() { + let root = tempfile::tempdir().unwrap(); + let directory = root.path().join("audit"); + let (layer, resolved) = super::open_ocsf_jsonl_layer(directory.clone()); + assert_eq!(resolved, Some(directory.clone())); + let subscriber = tracing_subscriber::registry().with(layer.expect("open audit sink")); + let ctx = EventContext { + sandbox_id: "portable-audit".into(), + sandbox_name: "portable-audit".into(), + container_image: String::new(), + hostname: "gateway-host".into(), + product_version: openshell_core::VERSION.into(), + proxy_ip: "127.0.0.1".parse().unwrap(), + proxy_port: 0, + origin: openshell_ocsf::EventOrigin::Supervisor, + }; + tracing::subscriber::with_default(subscriber, || { + emit_ocsf_event_routed("portable-audit", AppLifecycleBuilder::new(&ctx).build()); + }); + let files: Vec<_> = std::fs::read_dir(directory).unwrap().collect(); + assert_eq!(files.len(), 1); + let contents = std::fs::read_to_string(files[0].as_ref().unwrap().path()).unwrap(); + let records: Vec<_> = contents.lines().collect(); + assert_eq!(records.len(), 1); + assert!(serde_json::from_str::(records[0]).is_ok()); + } + + #[test] + fn portable_gateway_audit_sink_rejects_a_file_as_directory() { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join("file"); + std::fs::write(&path, b"not a directory").unwrap(); + let (layer, directory) = super::open_ocsf_jsonl_layer(path); + assert!(layer.is_none()); + assert!(directory.is_none()); + } #[derive(Clone)] struct SharedWriter(Arc>>); @@ -316,33 +342,20 @@ mod tests { #[test] fn gateway_ocsf_jsonl_requires_explicit_opt_in() { - assert!(!mxc_ocsf_jsonl_requested(Some("mxc"), None)); - assert!(!mxc_ocsf_jsonl_requested(Some("mxc"), Some(""))); - assert!(!mxc_ocsf_jsonl_requested(Some("mxc"), Some("enabled"))); + assert!(!ocsf_jsonl_requested(None)); + assert!(!ocsf_jsonl_requested(Some(""))); + assert!(!ocsf_jsonl_requested(Some("enabled"))); for value in ["0", "false", "FALSE", " off ", "no"] { - assert!(!mxc_ocsf_jsonl_requested(Some("mxc"), Some(value))); + assert!(!ocsf_jsonl_requested(Some(value))); } for value in ["1", "true", "TRUE", " on ", "yes"] { assert!( - mxc_ocsf_jsonl_requested(Some("mxc"), Some(value)), + ocsf_jsonl_requested(Some(value)), "expected {value:?} to opt in" ); } } - - #[test] - fn gateway_ocsf_jsonl_rejects_non_mxc_drivers() { - for driver in [ - None, - Some("docker"), - Some("kubernetes"), - Some("podman"), - Some("vm"), - ] { - assert!(!mxc_ocsf_jsonl_requested(driver, Some("1"))); - } - } } #[cfg(test)] diff --git a/crates/openshell-supervisor-network/src/host.rs b/crates/openshell-supervisor-network/src/host.rs index 7e29b3d0ed..5ae44dfff9 100644 --- a/crates/openshell-supervisor-network/src/host.rs +++ b/crates/openshell-supervisor-network/src/host.rs @@ -222,6 +222,7 @@ pub async fn start_host_proxy(config: HostProxyConfig) -> Result PathBuf { + if cfg!(target_os = "windows") && path.starts_with('/') { + PathBuf::from(format!("C:{path}")) + } else { + PathBuf::from(path) + } + } + fn supplied_identity( executable_path: &str, executable_digest: Option<&str>, @@ -300,13 +308,13 @@ mod tests { ) -> BinaryIdentity { BinaryIdentity { executable: ExecutableIdentity { - path: PathBuf::from(executable_path), + path: native_identity_path(executable_path), digest: executable_digest.map(digest), }, ancestors: ancestors .iter() .map(|(path, digest_value)| ExecutableIdentity { - path: PathBuf::from(path), + path: native_identity_path(path), digest: digest_value.map(digest), }) .collect(), @@ -450,7 +458,7 @@ mod tests { .hashes .lock() .unwrap() - .contains_key(Path::new("/sandbox/other")) + .contains_key(&native_identity_path("/sandbox/other")) ); } @@ -480,8 +488,8 @@ mod tests { assert!(error.contains("capacity")); let hashes = cache.hashes.lock().unwrap(); assert_eq!(hashes.len(), 4095); - assert!(!hashes.contains_key(Path::new("/sandbox/new-leaf"))); - assert!(!hashes.contains_key(Path::new("/sandbox/new-ancestor"))); + assert!(!hashes.contains_key(&native_identity_path("/sandbox/new-leaf"))); + assert!(!hashes.contains_key(&native_identity_path("/sandbox/new-ancestor"))); } #[test] diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index 7064cda008..83b62c0836 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -273,6 +273,7 @@ impl ProxyHandle { network_mediation_source: Option>, policy_dns_store: Option>, direct_listener_identity: Option, + required_proxy_authorization: Option>, ) -> Result { // Use override bind_addr, fall back to policy http_addr, then default // to loopback:3128. The default allows the proxy to function when no @@ -289,6 +290,11 @@ impl ProxyHandle { } let source_backed = network_mediation_source.is_some(); + if source_backed && required_proxy_authorization.is_some() { + return Err(miette::miette!( + "proxy authorization cannot be required for a network mediation source" + )); + } let listener = if source_backed { None } else { @@ -511,6 +517,7 @@ impl ProxyHandle { let dtx = denial_tx.clone(); let atx = activity_tx.clone(); let endpoint_observations = endpoint_observation_tx.clone(); + let required_authorization = required_proxy_authorization.clone(); tokio::spawn(async move { #[allow(clippy::large_futures)] if let Err(err) = handle_mediated_connection( @@ -534,6 +541,7 @@ impl ProxyHandle { dtx, atx, endpoint_observations, + required_authorization, ) .await { @@ -1555,7 +1563,6 @@ fn build_accept_error_event( .message(message) .build() } - fn classify_accept_error( err: &std::io::Error, consecutive_resource_errors: &mut u32, @@ -1563,7 +1570,6 @@ fn classify_accept_error( ) -> AcceptAction { #[cfg(not(unix))] let _ = (err, &mut *consecutive_resource_errors); - #[cfg(unix)] if matches!( err.raw_os_error(), @@ -2315,6 +2321,40 @@ where .await } +fn constant_time_bytes_eq(left: &[u8], right: &[u8]) -> bool { + let max_len = left.len().max(right.len()); + let mut difference = left.len() ^ right.len(); + for index in 0..max_len { + let left_byte = left.get(index).copied().unwrap_or_default(); + let right_byte = right.get(index).copied().unwrap_or_default(); + difference |= usize::from(left_byte ^ right_byte); + } + difference == 0 +} + +fn has_valid_proxy_authorization(request: &str, expected: &str) -> bool { + let mut provided = None; + for line in request.split("\r\n").skip(1) { + if line.is_empty() { + break; + } + let Some((name, value)) = line.split_once(':') else { + return false; + }; + if name.eq_ignore_ascii_case("proxy-authorization") { + // Reject duplicates even when both values are correct. Accepting + // ambiguous credentials can produce parser differentials between + // this proxy and downstream HTTP implementations. + if provided.is_some() { + return false; + } + provided = Some(value.trim()); + } + } + + provided.is_some_and(|value| constant_time_bytes_eq(value.as_bytes(), expected.as_bytes())) +} + // Many distinct, non-related context parameters are required for a CONNECT // dispatch; bundling them into a struct would just shift the noise into call // sites. @@ -2367,6 +2407,7 @@ async fn handle_tcp_connection( denial_tx, activity_tx, endpoint_observation_tx, + None, )) .await } @@ -2438,6 +2479,7 @@ async fn handle_mediated_connection( denial_tx: Option>, activity_tx: Option, endpoint_observation_tx: Option, + required_proxy_authorization: Option>, ) -> Result<()> { // Bind observations to the policy/provider inventory active when this // connection was accepted, even if configuration changes while it runs. @@ -2536,6 +2578,20 @@ async fn handle_mediated_connection( respond(&mut client, b"HTTP/1.1 400 Bad Request\r\n\r\n").await?; return Ok(()); } + if let Some(expected) = required_proxy_authorization.as_deref() + && !has_valid_proxy_authorization(request, expected) + { + warn!("Rejected host proxy request with missing or invalid per-sandbox credentials"); + respond( + &mut client, + b"HTTP/1.1 407 Proxy Authentication Required\r\n\ + Proxy-Authenticate: Basic realm=\"OpenShell\"\r\n\ + Content-Length: 0\r\n\ + Connection: close\r\n\r\n", + ) + .await?; + return Ok(()); + } let mut lines = request.split("\r\n"); let request_line = lines.next().unwrap_or(""); let mut parts = request_line.split_whitespace(); @@ -6934,17 +6990,60 @@ fn is_benign_relay_error(err: &miette::Report) -> bool { reason = "Test code: test fixtures and explicit control-flow markers are idiomatic in tests." )] mod tests { + #[test] + fn proxy_authorization_rejects_missing_wrong_cross_and_duplicate_credentials() { + let expected = "Basic sandbox-a-generation"; + let request = |headers: &str| format!("CONNECT example.com:443 HTTP/1.1\r\n{headers}\r\n"); + for headers in [ + "", + "Proxy-Authorization: Basic wrong\r\n", + "Proxy-Authorization: Basic sandbox-b-generation\r\n", + "Proxy-Authorization: Basic sandbox-a-generation\r\nProxy-Authorization: Basic sandbox-a-generation\r\n", + "Malformed-header\r\nProxy-Authorization: Basic sandbox-a-generation\r\n", + ] { + assert!(!has_valid_proxy_authorization(&request(headers), expected)); + } + assert!(has_valid_proxy_authorization( + &request("proxy-authorization: Basic sandbox-a-generation\r\n"), + expected, + )); + } + use super::*; use openshell_core::proposals::AgentProposals; use std::collections::HashMap as TestHashMap; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; + use std::path::Path; use std::sync::Arc; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::{TcpListener, TcpStream}; + // These contract tests use host-native absolute identities and matching + // policy declarations. Relative-path rejection remains independently tested. + fn native_identity_path(path: impl AsRef) -> PathBuf { + let path = path.as_ref(); + if cfg!(target_os = "windows") && path.to_string_lossy().starts_with('/') { + PathBuf::from(format!("C:{}", path.display())) + } else { + path.to_path_buf() + } + } + + fn native_identity_policy(source: &str) -> std::borrow::Cow<'_, str> { + if cfg!(target_os = "windows") { + std::borrow::Cow::Owned(source.replace("path: /", "path: C:/")) + } else { + std::borrow::Cow::Borrowed(source) + } + } + + fn native_identity_engine(rego: &str, source: &str) -> Result { + OpaEngine::from_strings(rego, &native_identity_policy(source)) + } + #[test] fn supplied_identity_preserves_authorized_endpoint_metadata() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), r#" network_policies: @@ -6975,7 +7074,7 @@ process: .expect("load policy"); let identity = Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/usr/bin/python3"), + path: native_identity_path("/usr/bin/python3"), digest: Some("00".repeat(32).parse().expect("digest")), }, ancestors: Vec::new(), @@ -7001,7 +7100,7 @@ process: #[test] fn supplied_identity_rejects_same_path_replacement() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), r#" network_policies: @@ -7031,7 +7130,7 @@ process: let identity = |digest_byte: &str| { Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/sandbox/bin/client"), + path: native_identity_path("/sandbox/bin/client"), digest: Some(digest_byte.repeat(32).parse().expect("digest")), }, ancestors: Vec::new(), @@ -7058,7 +7157,7 @@ process: #[test] fn supplied_identity_rejects_replaced_authorizing_ancestor() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), r#" network_policies: @@ -7084,11 +7183,11 @@ process: let identity = |ancestor_digest: &str| { Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/sandbox/bin/client"), + path: native_identity_path("/sandbox/bin/client"), digest: Some("11".repeat(32).parse().expect("digest")), }, ancestors: vec![ContractExecutableIdentity { - path: PathBuf::from("/sandbox/bin/launcher"), + path: native_identity_path("/sandbox/bin/launcher"), digest: Some(ancestor_digest.repeat(32).parse().expect("digest")), }], cmdline_paths: Vec::new(), @@ -7131,11 +7230,11 @@ process: run_as_group: sandbox "#; let rego = include_str!("../data/sandbox-policy.rego"); - let engine = OpaEngine::from_strings(rego, POLICY_DATA).expect("load policy"); + let engine = native_identity_engine(rego, POLICY_DATA).expect("load policy"); let identity = |digest_byte: &str| { Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/sandbox/bin/client"), + path: native_identity_path("/sandbox/bin/client"), digest: Some(digest_byte.repeat(32).parse().expect("digest")), }, ancestors: Vec::new(), @@ -7149,7 +7248,9 @@ process: authorize_supplied_identity(&engine, &identity_cache, intent(), &identity("11")); assert!(matches!(original.action, NetworkAction::Allow { .. })); - engine.reload(rego, POLICY_DATA).expect("reload policy"); + engine + .reload(rego, &native_identity_policy(POLICY_DATA)) + .expect("reload policy"); let replacement = authorize_supplied_identity(&engine, &identity_cache, intent(), &identity("22")); @@ -7163,7 +7264,7 @@ process: #[test] fn supplied_identity_rejects_missing_ancestor_digest_before_policy() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), r#" network_policies: @@ -7188,11 +7289,11 @@ process: .expect("load policy"); let identity = Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/sandbox/bin/client"), + path: native_identity_path("/sandbox/bin/client"), digest: Some("11".repeat(32).parse().expect("digest")), }, ancestors: vec![ContractExecutableIdentity { - path: PathBuf::from("/sandbox/bin/launcher"), + path: native_identity_path("/sandbox/bin/launcher"), digest: None, }], cmdline_paths: Vec::new(), @@ -7214,7 +7315,7 @@ process: #[tokio::test] async fn staged_transparent_open_waits_for_l4_policy() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), r#" network_policies: @@ -7244,7 +7345,7 @@ process: let identity = || { Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/usr/bin/curl"), + path: native_identity_path("/usr/bin/curl"), digest: Some("00".repeat(32).parse().unwrap()), }, ancestors: Vec::new(), @@ -7337,7 +7438,10 @@ process: .expect("policy denial is sent to mapper"); assert_eq!(event.host, "203.0.113.8"); assert_eq!(event.port, 443); - assert_eq!(event.binary, "/usr/bin/curl"); + assert_eq!( + event.binary, + native_identity_path("/usr/bin/curl").display().to_string() + ); assert_eq!(event.denial_stage, "transparent_tcp_connect"); assert!(denial_rx.try_recv().is_err(), "exactly one mapper event"); } @@ -7439,7 +7543,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } stream: Box::new(stream), binary_identity: Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/usr/bin/curl"), + path: native_identity_path("/usr/bin/curl"), digest: Some("00".repeat(32).parse().unwrap()), }, ancestors: Vec::new(), @@ -7514,6 +7618,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } None, None, None, + None, )) .await .unwrap(); @@ -7622,7 +7727,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } #[tokio::test] async fn staged_transparent_open_dials_only_pinned_policy_dns_addresses() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), POLICY_DNS_OPEN_POLICY, ) @@ -7662,7 +7767,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } #[tokio::test] async fn staged_transparent_open_proposes_a_denied_observation_hostname() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), POLICY_DNS_OPEN_POLICY, ) @@ -7697,12 +7802,15 @@ process: { run_as_user: sandbox, run_as_group: sandbox } let event = denial_rx.try_recv().expect("denial is sent to the mapper"); assert_eq!(event.host, "unknown.example"); assert_eq!(event.port, 443); - assert_eq!(event.binary, "/usr/bin/curl"); + assert_eq!( + event.binary, + native_identity_path("/usr/bin/curl").display().to_string() + ); } #[tokio::test] async fn staged_transparent_open_never_relays_an_observation_that_policy_allows() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), POLICY_DNS_OPEN_POLICY, ) @@ -7768,7 +7876,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } #[test] fn pinned_plan_requires_the_mapping_of_the_deciding_generation() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), POLICY_DNS_OPEN_POLICY, ) @@ -7813,7 +7921,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } engine .reload( include_str!("../data/sandbox-policy.rego"), - POLICY_DNS_OPEN_POLICY, + &native_identity_policy(POLICY_DNS_OPEN_POLICY), ) .unwrap(); let reloaded = decide("db.example", 5432); @@ -7836,7 +7944,10 @@ process: { run_as_user: sandbox, run_as_group: sandbox } ); assert_eq!( mapped.format_shorthand(), - "NET:OPEN [MED] DENIED /usr/bin/curl(0) -> blocked.invalid:80 [reason:transparent_tcp_policy_denied]" + format!( + "NET:OPEN [MED] DENIED {}(0) -> blocked.invalid:80 [reason:transparent_tcp_policy_denied]", + native_identity_path("/usr/bin/curl").display() + ) ); assert_eq!( serde_json::to_value(mapped).unwrap()["dst_endpoint"], @@ -7859,7 +7970,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } #[tokio::test] async fn staged_policy_local_open_reaches_the_sandbox_scoped_api() { let engine = Arc::new( - OpaEngine::from_strings( + native_identity_engine( include_str!("../data/sandbox-policy.rego"), "network_policies: {}\n", ) @@ -7868,7 +7979,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } let cache = Arc::new(BinaryIdentityCache::new()); let identity = ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/usr/bin/bash"), + path: native_identity_path("/usr/bin/bash"), digest: Some("44".repeat(32).parse().unwrap()), }, ancestors: Vec::new(), @@ -7928,6 +8039,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } None, None, None, + None, )); workload .write_all(b"GET /v1/policy/current HTTP/1.1\r\nHost: policy.local\r\nConnection: close\r\n\r\n") @@ -7978,7 +8090,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } .unwrap(); }); let engine = Arc::new( - OpaEngine::from_strings( + native_identity_engine( include_str!("../data/sandbox-policy.rego"), &format!( r#" @@ -8004,7 +8116,7 @@ network_policies: stream: Box::new(stream), binary_identity: Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/usr/bin/curl"), + path: native_identity_path("/usr/bin/curl"), digest: Some("44".repeat(32).parse().unwrap()), }, ancestors: Vec::new(), @@ -8056,6 +8168,7 @@ network_policies: None, None, None, + None, )); let request = tokio::time::timeout(std::time::Duration::from_secs(5), request_rx) .await @@ -8182,7 +8295,7 @@ process: #[tokio::test] async fn staged_transparent_open_reports_identity_cache_capacity_exhaustion() { - let engine = OpaEngine::from_strings( + let engine = native_identity_engine( include_str!("../data/sandbox-policy.rego"), r#" network_policies: @@ -8210,7 +8323,7 @@ process: identity_cache .verify_or_cache_supplied_identity(&ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from(format!("/sandbox/pinned-{index}")), + path: native_identity_path(format!("/sandbox/pinned-{index}")), digest: Some("11".repeat(32).parse().unwrap()), }, ancestors: Vec::new(), @@ -8224,7 +8337,7 @@ process: stream: Box::new(stream), binary_identity: Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { - path: PathBuf::from("/sandbox/overflow"), + path: native_identity_path("/sandbox/overflow"), digest: Some("22".repeat(32).parse().unwrap()), }, ancestors: Vec::new(), @@ -8662,6 +8775,7 @@ network_policies: {} Some(Arc::new(FailedMediationSource)), None, None, + None, ) .await .expect("proxy starts before source accept"); diff --git a/crates/openshell-supervisor-network/src/run.rs b/crates/openshell-supervisor-network/src/run.rs index 1eba8a0742..51418d62e4 100644 --- a/crates/openshell-supervisor-network/src/run.rs +++ b/crates/openshell-supervisor-network/src/run.rs @@ -40,6 +40,25 @@ use crate::proxy::ProxyHandle; use openshell_core::endpoint_status::EndpointObservationSender; use openshell_isolation_interface::contract::NetworkMediationSource; +/// Concrete listener options supplied by trusted supervisor composition. +/// This is not an isolation-backend capability or a serialized launch contract. +#[derive(Clone)] +pub struct ProxyListenerConfig { + pub bind_addr: SocketAddr, + pub authorization: Arc, + pub binary_identity: openshell_isolation_interface::contract::BinaryIdentity, +} + +impl std::fmt::Debug for ProxyListenerConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ProxyListenerConfig") + .field("bind_addr", &self.bind_addr) + .field("authorization", &"") + .field("binary_identity", &self.binary_identity) + .finish() + } +} + #[cfg(target_os = "linux")] pub struct TransparentRuntimeSetup { pub listeners: Vec, @@ -201,6 +220,7 @@ pub async fn run_networking( host_gateway_ip: Option, #[cfg(target_os = "linux")] transparent_runtime: Option, network_mediation_source: Option>, + proxy_listener: Option, ) -> Result { // Build the policy-local route context. The orchestrator's policy poll // loop also holds an `Arc` clone (via `Networking::policy_local_ctx`) so @@ -467,10 +487,15 @@ pub async fn run_networking( // originating inside the namespace can reach the proxy. Otherwise the // proxy falls back to the policy-declared http_addr (loopback in // tests, etc.). - let bind_addr = proxy_bind_ip.map(|ip| { - let port = proxy_policy.http_addr.map_or(3128, |addr| addr.port()); - SocketAddr::new(ip, port) - }); + let bind_addr = proxy_listener + .as_ref() + .map(|proxy| proxy.bind_addr) + .or_else(|| { + proxy_bind_ip.map(|ip| { + let port = proxy_policy.http_addr.map_or(3128, |addr| addr.port()); + SocketAddr::new(ip, port) + }) + }); let proxy_handle = ProxyHandle::start_with_bind_addr( proxy_policy, @@ -491,7 +516,12 @@ pub async fn run_networking( mediated_policy_dns .as_ref() .map(|runtime| runtime.store.clone()), - None, + proxy_listener + .as_ref() + .map(|proxy| proxy.binary_identity.clone()), + proxy_listener + .as_ref() + .map(|proxy| proxy.authorization.clone()), ) .await?; Some(proxy_handle) diff --git a/crates/openshell-supervisor-process/Cargo.toml b/crates/openshell-supervisor-process/Cargo.toml index a7cd29ba8b..41039b2000 100644 --- a/crates/openshell-supervisor-process/Cargo.toml +++ b/crates/openshell-supervisor-process/Cargo.toml @@ -21,7 +21,6 @@ base64 = { workspace = true } bytes = { workspace = true } hex = "0.4" miette = { workspace = true } -nix = { workspace = true } rand = "0.10" russh = "0.63.1" serde_json = { workspace = true } 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..80759131d1 100644 --- a/crates/openshell-supervisor-process/src/main_session.rs +++ b/crates/openshell-supervisor-process/src/main_session.rs @@ -4,15 +4,10 @@ //! Retained I/O multiplexer for the canonical sandbox process. use std::collections::VecDeque; -use std::io::{Read, Write}; -use std::os::fd::AsRawFd; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use bytes::Bytes; -use nix::fcntl::{FcntlArg, OFlag, fcntl}; -use nix::pty::Winsize; -use tokio::io::unix::AsyncFd; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::sync::Notify; use tokio::sync::watch; @@ -23,16 +18,6 @@ use openshell_isolation_interface::contract::{ const OUTPUT_BUFFER_BYTES: usize = 1024 * 1024; -/// Canonical-process I/O retained by the supervisor session multiplexer. -pub enum ProcessIo { - Pty(std::fs::File), - Pipes { - stdin: tokio::process::ChildStdin, - stdout: tokio::process::ChildStdout, - stderr: tokio::process::ChildStderr, - }, -} - #[derive(Clone, Debug)] pub enum MainOutput { Stdout(Bytes), @@ -191,13 +176,11 @@ impl MainOutputCursor { } pub struct MainSession { - pid: u32, terminal: bool, input: tokio::sync::mpsc::Sender>, output: Arc, input_owner: Mutex>, next_owner: AtomicU64, - pty_master: Option>, boundary_process: Option>, boundary_terminal: Option>, readers_remaining: AtomicUsize, @@ -218,13 +201,11 @@ 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 { - pid: 1, terminal: false, input, output: OutputLog::new(), input_owner: Mutex::new(None), next_owner: AtomicU64::new(1), - pty_master: None, boundary_process: None, boundary_terminal: None, readers_remaining: AtomicUsize::new(0), @@ -240,61 +221,6 @@ impl MainSession { (session, input_rx) } - #[cfg(test)] - 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); - ( - Self::new(ProcessIo::Pty(std::fs::File::from(pty.master)), 1), - slave, - ) - } - - #[cfg(test)] - #[allow(unsafe_code)] - pub fn terminal_size_for_test(&self) -> (u16, u16) { - let master = self.pty_master.as_ref().expect("terminal PTY master"); - let mut winsize: libc::winsize = unsafe { std::mem::zeroed() }; - let result = unsafe { libc::ioctl(master.as_raw_fd(), libc::TIOCGWINSZ, &mut winsize) }; - assert_eq!(result, 0, "read terminal dimensions"); - (winsize.ws_col, winsize.ws_row) - } - - #[must_use] - pub fn new(io: ProcessIo, pid: u32) -> Arc { - let terminal = matches!(io, ProcessIo::Pty(_)); - let (input, input_rx) = tokio::sync::mpsc::channel::>(64); - let pty_master = match &io { - ProcessIo::Pty(master) => { - set_nonblocking(master).expect("set canonical PTY master nonblocking"); - master.try_clone().ok().map(Arc::new) - } - ProcessIo::Pipes { .. } => None, - }; - let session = Arc::new(Self { - pid, - terminal, - input, - output: OutputLog::new(), - input_owner: Mutex::new(None), - next_owner: AtomicU64::new(1), - pty_master, - boundary_process: None, - boundary_terminal: None, - readers_remaining: AtomicUsize::new(if terminal { 1 } else { 2 }), - readers_done: Notify::new(), - finished: std::sync::atomic::AtomicBool::new(false), - terminal_attachments: Mutex::new(TerminalAttachmentState { - active: 0, - process_finished: false, - expectation: AttachmentExpectation::None, - }), - terminal_attachments_done: Notify::new(), - }); - Self::start_io(&session, io, input_rx); - session - } - /// Build the control-side multiplexer around a boundary-owned admitted /// process. Process lifecycle and PTY operations remain delegated to the /// boundary process handle. @@ -312,13 +238,11 @@ impl MainSession { let terminal_mode = terminal.is_some(); let (input, mut input_rx) = tokio::sync::mpsc::channel::>(64); let session = Arc::new(Self { - pid: 0, terminal: terminal_mode, input, output: OutputLog::new(), input_owner: Mutex::new(None), next_owner: AtomicU64::new(1), - pty_master: None, boundary_process: Some(process), boundary_terminal: terminal, readers_remaining: AtomicUsize::new(if terminal_mode { 1 } else { 2 }), @@ -370,99 +294,9 @@ impl MainSession { session } - fn start_io( - this: &Arc, - io: ProcessIo, - mut input_rx: tokio::sync::mpsc::Receiver>, - ) { - match io { - ProcessIo::Pty(master) => { - let master = Arc::new(AsyncFd::new(master).expect("register canonical PTY master")); - let reader = Arc::clone(&master); - let output = Arc::clone(this); - tokio::spawn(async move { - let mut buffer = [0u8; 4096]; - loop { - let Ok(mut ready) = reader.readable().await else { - break; - }; - match ready.try_io(|inner| { - let mut file = inner.get_ref(); - file.read(&mut buffer) - }) { - Ok(Ok(0) | Err(_)) => break, - Ok(Ok(read)) => output.publish(MainOutput::Stdout( - Bytes::copy_from_slice(&buffer[..read]), - )), - Err(_would_block) => {} - } - } - output.reader_finished(); - }); - tokio::spawn(async move { - while let Some(data) = input_rx.recv().await { - let mut remaining = data.as_slice(); - while !remaining.is_empty() { - let Ok(mut ready) = master.writable().await else { - return; - }; - match ready.try_io(|inner| { - let mut file = inner.get_ref(); - file.write(remaining) - }) { - Ok(Ok(0) | Err(_)) => return, - Ok(Ok(written)) => remaining = &remaining[written..], - Err(_would_block) => {} - } - } - } - }); - } - ProcessIo::Pipes { - mut stdin, - mut stdout, - mut stderr, - } => { - let stdout_session = Arc::clone(this); - tokio::spawn(async move { - let mut buffer = [0u8; 4096]; - loop { - match stdout.read(&mut buffer).await { - Ok(0) | Err(_) => break, - Ok(read) => { - stdout_session.publish(MainOutput::Stdout(Bytes::copy_from_slice( - &buffer[..read], - ))); - } - } - } - stdout_session.reader_finished(); - }); - let stderr_session = Arc::clone(this); - tokio::spawn(async move { - let mut buffer = [0u8; 4096]; - loop { - match stderr.read(&mut buffer).await { - Ok(0) | Err(_) => break, - Ok(read) => { - stderr_session.publish(MainOutput::Stderr(Bytes::copy_from_slice( - &buffer[..read], - ))); - } - } - } - stderr_session.reader_finished(); - }); - tokio::spawn(async move { - while let Some(data) = input_rx.recv().await { - if stdin.write_all(&data).await.is_err() { - break; - } - let _ = stdin.flush().await; - } - }); - } - } + #[cfg(all(test, unix))] + pub(crate) fn publish_test_output(&self, data: &'static [u8]) { + self.publish(MainOutput::Stdout(Bytes::from_static(data))); } fn publish(&self, event: MainOutput) { @@ -645,7 +479,7 @@ impl MainSession { } } - pub async fn resize(&self, columns: u32, rows: u32, pixel_width: u32, pixel_height: u32) { + pub async fn resize(&self, columns: u32, rows: u32, _pixel_width: u32, _pixel_height: u32) { if let Some(terminal) = self.boundary_terminal.as_ref() { let _ = terminal .resize( @@ -653,39 +487,18 @@ impl MainSession { u16::try_from(rows.max(1)).unwrap_or(u16::MAX), ) .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); } } - 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 { - 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 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) + /// Delegate process-group signaling to the isolation backend. + pub async fn signal_boundary_group(&self, signal: BoundarySignal) -> Result<(), String> { + let process = self + .boundary_process + .as_ref() + .ok_or_else(|| "no boundary process is attached".to_string())?; + process + .signal(signal) + .await .map_err(|error| error.to_string()) } @@ -700,16 +513,6 @@ impl MainSession { } } -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); - fcntl( - file.as_raw_fd(), - FcntlArg::F_SETFL(flags | OFlag::O_NONBLOCK), - )?; - Ok(()) -} - #[cfg(test)] mod tests { use super::*; @@ -785,7 +588,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]); @@ -956,43 +759,14 @@ mod tests { } #[tokio::test] - async fn terminal_pump_reads_output_and_writes_input() { - let (session, mut slave) = MainSession::terminal_for_test(); - set_nonblocking(&slave).expect("set test PTY slave nonblocking"); - let mut output = session.subscribe(); - - slave - .write_all(b"process output") - .expect("write PTY output"); - let event = tokio::time::timeout(std::time::Duration::from_secs(1), output.recv()) - .await - .expect("PTY output timed out") - .expect("PTY output was retained"); - assert!(matches!( - event, - MainOutput::Stdout(data) if data == b"process output"[..] - )); - - let (owner, input) = session.acquire_input().expect("acquire PTY input"); - input - .send(b"client input\n".to_vec()) - .await - .expect("queue PTY input"); - let mut received = [0; 64]; - let read = tokio::time::timeout(std::time::Duration::from_secs(1), async { - loop { - match slave.read(&mut received) { - Ok(read) => break read, - Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { - tokio::task::yield_now().await; - } - Err(error) => panic!("read PTY input: {error}"), - } - } - }) - .await - .expect("PTY input timed out"); - assert_eq!(&received[..read], b"client input\n"); - session.release_input(owner); + async fn signaling_without_boundary_is_rejected() { + let session = MainSession::inert(); + assert_eq!( + session + .signal_boundary_group(BoundarySignal::Term) + .await + .unwrap_err(), + "no boundary process is attached" + ); } } diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 928571fd50..20d37585ce 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -954,15 +954,14 @@ impl russh::server::Handler for SshHandler { .is_some_and(|state| state.main_attached) { let signal = match signal { - Sig::HUP => Some(nix::sys::signal::Signal::SIGHUP), - Sig::INT => Some(nix::sys::signal::Signal::SIGINT), - Sig::KILL => Some(nix::sys::signal::Signal::SIGKILL), - Sig::QUIT => Some(nix::sys::signal::Signal::SIGQUIT), - Sig::TERM => Some(nix::sys::signal::Signal::SIGTERM), + Sig::HUP => Some(openshell_isolation_interface::contract::BoundarySignal::Hup), + Sig::INT => Some(openshell_isolation_interface::contract::BoundarySignal::Int), + Sig::KILL => Some(openshell_isolation_interface::contract::BoundarySignal::Kill), + Sig::TERM => Some(openshell_isolation_interface::contract::BoundarySignal::Term), _ => None, }; if let (Some(signal), Some(main_session)) = (signal, self.main_session.as_ref()) - && let Err(error) = main_session.signal_group(signal).await + && let Err(error) = main_session.signal_boundary_group(signal).await { warn!(%error, ?signal, "failed to signal canonical main process group"); } @@ -1315,7 +1314,6 @@ fn direct_tcpip_target( )] mod tests { use super::*; - use std::io::Write as _; pub(super) struct AcceptAnyServerKey; @@ -1792,14 +1790,13 @@ mod tests { #[tokio::test] async fn main_attachment_accepts_declared_session_after_process_exit() { - let (main_session, mut slave) = MainSession::terminal_for_test(); + let main_session = MainSession::inert(); let mut output = main_session.subscribe(); - slave.write_all(b"retained output").unwrap(); + main_session.publish_test_output(b"retained output"); assert!(matches!( tokio::time::timeout(Duration::from_secs(5), output.recv()).await.unwrap().unwrap(), MainOutput::Stdout(data) if data == b"retained output"[..] )); - drop(slave); assert!( tokio::time::timeout(Duration::from_secs(5), main_session.finish(23, true)) .await diff --git a/crates/openshell-supervisor-process/src/supervisor_session.rs b/crates/openshell-supervisor-process/src/supervisor_session.rs index 607ddee204..e38783d47b 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>, @@ -851,22 +851,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)) } } } @@ -974,10 +984,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( @@ -1004,6 +1012,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 index 6ecbfb6059..60c3d05e75 100644 --- a/crates/openshell-supervisor/README.md +++ b/crates/openshell-supervisor/README.md @@ -15,3 +15,29 @@ The supervisor admits policy and prepares credentials before constructing and at The client receives the supervisor's live provider state, bearer-token slot, and CA-path slot. Provider refresh, token rotation, and later CA publication must remain visible through those shared handles. Startup does not create independent copies of their current values. The setup interface stays private to the supervisor. It adds no runtime backend registration, endpoint configuration, or public factory API. The public `run_sandbox` signature and standard backend selection remain unchanged. + +The private startup result separates the backend transport payload from optional +authenticated CONNECT listener settings. Shared networking starts the listener +only after boundary attachment and confirmation; the supervisor owns policy +evaluation, live credentials, and listener lifetime. These settings do not add +a proxy hook to the isolation interface or the shared Sandbox Protocol descriptor. + +## Portable supervisor access + +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. +The process-access multiplexer consumes boundary-provided streams and delegates +terminal resizing and signals through the existing isolation contract. It does +not own local PIDs, PTYs, or process-group signaling; native process operations +belong to the sandbox or isolation backend. Signals use `BoundarySignal`. + +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/backend_setup.rs b/crates/openshell-supervisor/src/backend_setup.rs index 61fe5a71f0..c312ca6a36 100644 --- a/crates/openshell-supervisor/src/backend_setup.rs +++ b/crates/openshell-supervisor/src/backend_setup.rs @@ -36,6 +36,14 @@ pub struct BackendServices { pub sandbox_bearer: SessionBearerTokenSlot, } +/// Private startup result; native payload decoding never reaches shared runtime +/// descriptors or the public isolation interface. +pub struct BuiltBackend { + pub backend: Arc, + pub payload: Vec, + pub proxy_listener: Option, +} + /// Selected by trusted composition, never by payload contents. Decoding and /// construction may prepare a client but must not launch workload code. pub trait BackendSetup: Sync { @@ -66,7 +74,7 @@ pub trait PreparedBackend: Send + Sync { fn build( self: Box, services: BackendServices, - ) -> std::result::Result, BackendError>; + ) -> std::result::Result; } /// Created only after name and launch identity checks. Consuming attachment @@ -153,24 +161,28 @@ impl SelectedBackend { /// Construct and attach the selected client using the admitted policy and /// shared services. Registry verification rejects a differently named client. pub async fn attach( - self, + mut self, services: BackendServices, policy: SandboxPolicy, agent: AgentSpec, - ) -> Result> { - let backend = self + ) -> Result<( + Box, + Option, + )> { + let built = self .prepared .build(services) .map_err(|error| miette::miette!(error.to_string()))?; + self.descriptor.payload = built.payload; let mut registry = BackendRegistry::new(); registry - .register(backend) + .register(built.backend) .map_err(|error| miette::miette!(error.to_string()))?; let admitted_backend = self.descriptor.backend_name.clone(); let (backend, verified) = registry .resolve(self.descriptor, &admitted_backend) .map_err(|error| miette::miette!(error.to_string()))?; - backend + let bound = backend .attach( verified, SandboxContext { @@ -182,7 +194,8 @@ impl SelectedBackend { }, ) .await - .map_err(|error| miette::miette!(error.to_string())) + .map_err(|error| miette::miette!(error.to_string()))?; + Ok((bound, built.proxy_listener)) } } @@ -235,14 +248,17 @@ impl PreparedBackend for OpenShellLaunch { fn build( self: Box, services: BackendServices, - ) -> std::result::Result, BackendError> { - Ok(Arc::new( - openshell_sandbox_backend::OpenShellRuntimeBackend::new( + ) -> std::result::Result { + let payload = self.0.backend_descriptor()?.payload; + Ok(BuiltBackend { + payload, + proxy_listener: None, + backend: Arc::new(openshell_sandbox_backend::OpenShellRuntimeBackend::new( services.ca_file_paths, services.provider_credentials, services.sandbox_bearer, - ), - )) + )), + }) } } diff --git a/crates/openshell-supervisor/src/backend_setup/tests.rs b/crates/openshell-supervisor/src/backend_setup/tests.rs index fa4ab65ba5..a4328776c7 100644 --- a/crates/openshell-supervisor/src/backend_setup/tests.rs +++ b/crates/openshell-supervisor/src/backend_setup/tests.rs @@ -16,6 +16,7 @@ use openshell_isolation_interface::contract::{ NetworkMediationSource, OuterFenceGuarantee, OuterFenceGuarantees, PendingDnsQuery, PendingTcpOpen, ReadyBoundary, RunningBoundary, VerifiedBackendDescriptor, }; +#[cfg(unix)] use openshell_supervisor_network::upstream_proxy::UpstreamProxyArgs; const TEST_BACKEND: &str = "in-process-test"; @@ -184,17 +185,21 @@ impl PreparedBackend for TestLaunch { fn build( self: Box, services: BackendServices, - ) -> std::result::Result, BackendError> { + ) -> std::result::Result { self.observed.record("build"); let services = Arc::new(services); *self.observed.services.lock().unwrap() = Arc::downgrade(&services); - Ok(Arc::new(TestBackend { - name: self.name, - observed: self.observed, - services, - generation: self.generation, - expected_session: self.expected_session, - })) + Ok(BuiltBackend { + payload: TEST_PAYLOAD.to_vec(), + proxy_listener: None, + backend: Arc::new(TestBackend { + name: self.name, + observed: self.observed, + services, + generation: self.generation, + expected_session: self.expected_session, + }), + }) } } @@ -466,7 +471,8 @@ async fn attachment_receives_live_supervisor_services() { .select() .attach(services, policy(), agent()) .await - .unwrap(); + .unwrap() + .0; let backend_services = setup.observed.services.lock().unwrap().upgrade().unwrap(); assert!(Arc::ptr_eq(&ca_paths, &backend_services.ca_file_paths)); assert!(Arc::ptr_eq( diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index fb14b5ec99..bbd3539fbc 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -101,21 +101,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) @@ -127,7 +145,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. } } } @@ -154,6 +175,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), } @@ -170,9 +192,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()) @@ -282,6 +302,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(); @@ -573,6 +600,7 @@ pub async fn run_network_proxy( #[cfg(target_os = "linux")] None, None, + None, ) .await?; @@ -899,7 +927,7 @@ async fn run_sandbox_with_backend( // supervisor-owned handles shared with the backend. let admitted_backend_name = selected_backend.backend_name().to_string(); let ca_file_paths = Arc::new(std::sync::Mutex::new(None)); - let bound = selected_backend + let (bound, proxy_listener) = selected_backend .attach( backend_setup::BackendServices { ca_file_paths: ca_file_paths.clone(), @@ -982,7 +1010,9 @@ async fn run_sandbox_with_backend( // API read the current value so proposals target the correct workspace. let (workspace_tx, workspace_rx) = tokio::sync::watch::channel(String::new()); - let remote_network_source = remote_boundary.0.network_mediation_source(); + let remote_network_source = proxy_listener + .is_none() + .then(|| remote_boundary.0.network_mediation_source()); let remote_host_gateway_ip = remote_boundary.0.host_gateway_ip(); let (remote_ready, backend_name, ca_file_paths) = { let (bound, backend_name, ca_file_paths) = remote_boundary; @@ -1020,7 +1050,8 @@ async fn run_sandbox_with_backend( remote_host_gateway_ip, #[cfg(target_os = "linux")] None, - Some(remote_network_source), + remote_network_source, + proxy_listener, ) .await?, ); @@ -1403,7 +1434,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`. @@ -5157,6 +5192,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(); @@ -5170,6 +5206,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(); @@ -5236,12 +5273,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; } }) @@ -5250,7 +5294,7 @@ mod tests { session_tx.send_replace(true); timeout(Duration::from_secs(1), async { - while !connects() { + while !connects().await { tokio::task::yield_now().await; } }) @@ -5259,7 +5303,7 @@ mod tests { drop(readiness); timeout(Duration::from_secs(1), async { - while connects() { + while connects().await { tokio::task::yield_now().await; } }) @@ -5267,7 +5311,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..8d0fb5ba38 100644 --- a/crates/openshell-supervisor/src/main.rs +++ b/crates/openshell-supervisor/src/main.rs @@ -176,14 +176,16 @@ fn arm_parent_liveness(raw_fd: Option) -> Result<()> { Ok(()) } -fn backend_descriptor(args: &Args) -> Result { +fn backend_descriptor(args: &Args, admitted_backend: Option<&str>) -> Result { let path = args.backend_descriptor_file.as_deref().ok_or_else(|| { miette::miette!("--backend-descriptor-file is required for --role=isolation-backend") })?; let payload = std::fs::read(path) .map_err(|error| miette::miette!("read backend descriptor {}: {error}", path.display()))?; Ok(BackendDescriptor { - backend_name: openshell_sandbox_backend::BACKEND_NAME.to_string(), + backend_name: admitted_backend + .unwrap_or(openshell_sandbox_backend::BACKEND_NAME) + .to_string(), payload, }) } @@ -298,8 +300,10 @@ fn main() -> Result<()> { validate_role_arguments(&args)?; arm_parent_liveness(args.parent_liveness_fd)?; validate_main_exit_marker(args.main_exit_marker.as_deref())?; + let admitted_isolation_backend = + std::env::var(openshell_core::sandbox_env::ADMITTED_ISOLATION_BACKEND).ok(); let isolation_inputs = if args.role == SupervisorRole::IsolationBackend { - let descriptor = backend_descriptor(&args)?; + let descriptor = backend_descriptor(&args, admitted_isolation_backend.as_deref())?; let auth = auth_bundle(&args)?; // Install the driver-provisioned session before starting log push or // any other gateway client. `run_sandbox` obtains the same Sandbox @@ -429,8 +433,6 @@ fn main() -> Result<()> { "isolation-backend role started without validated runtime inputs" )); }; - let admitted_isolation_backend = - std::env::var(openshell_core::sandbox_env::ADMITTED_ISOLATION_BACKEND).ok(); Box::pin(openshell_supervisor::run_sandbox( command, workdir, @@ -502,7 +504,14 @@ mod tests { assert_eq!(args.role, SupervisorRole::IsolationBackend); assert!(validate_role_arguments(&args).is_ok()); assert_eq!( - backend_descriptor(&args) + backend_descriptor(&args, None).unwrap().backend_name, + openshell_sandbox_backend::BACKEND_NAME + ); + let admitted = backend_descriptor(&args, Some("openshell-test-backend")).unwrap(); + assert_eq!(admitted.backend_name, "openshell-test-backend"); + assert_eq!(admitted.payload, vec![0]); + assert_eq!( + backend_descriptor(&args, None) .expect("runtime descriptor") .payload, vec![0] @@ -564,6 +573,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/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index c44101150b..f63a8f8426 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -920,6 +920,12 @@ administrator or belong to the Windows Performance Log Users group. Workload commands and working directories remain sandbox-scoped and must be supplied in the `mxc` driver configuration when creating a sandbox. +`OPENSHELL_OCSF_JSON=1` enables the gateway's JSONL audit +sink independently of the selected compute driver. Set `OPENSHELL_OCSF_LOG_DIR` +to override the default `%PROGRAMDATA%\OpenShell\logs` directory on Windows +or `$XDG_STATE_HOME/openshell/logs` on Unix. Prefer the gateway's configured +`ocsf_log` output for retention controls and loss metrics. + The driver records executable identity in process audit events and omits raw command arguments because they can contain credentials or personal data. See [OCSF JSON Export](/observability/ocsf-json-export) for durable Windows audit diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index 5be0d554ba..ffdc16bd70 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -387,6 +387,16 @@ openshell sandbox create --env API_KEY=sk-test --env DEBUG=1 -- my-agent Variables set with `--env` are available to all processes in the sandbox, including the initial command, interactive shells, and exec commands. +Use `--env-from KEY[=ENVVAR]` when the value should come from the CLI process environment instead of appearing in the CLI process arguments. If `ENVVAR` is omitted, OpenShell reads `KEY`: + +```shell +export SESSION_TOKEN="..." +openshell sandbox create --env-from SESSION_TOKEN -- my-agent +openshell sandbox create --env-from AGENT_TOKEN=SESSION_TOKEN -- my-agent +``` + +`--env-from` changes only how the CLI receives the value. The resulting variable is still available to processes in the sandbox, just like `--env`. + When an `--env` key looks like a credential — a known provider variable, or a name whose underscore-separated segments include a credential word such as `TOKEN`, `SECRET`, `PASSWORD`, `CREDENTIAL`, `API_KEY`, `ACCESS_KEY`, or `SECRET_KEY` (for example `DB_TOKEN` or `MY_ACCESS_KEY`) — `sandbox create` prints a non-blocking warning. Matching is on whole segments, so unrelated names like `TOKENIZERS_PARALLELISM` or `PASSWORDLESS_LOGIN` do not warn. The agent inside the sandbox can read plain environment values directly, so to hide a secret from the agent, attach it through a [profile-backed provider](/how-it-works/providers/profiles) with `--provider` instead. Suppress the warning with `--no-credential-warnings`. Detection uses the key name only; values are never inspected or printed. You can also set per-command environment variables with `sandbox exec`: diff --git a/docs/observability/ocsf-json-export.mdx b/docs/observability/ocsf-json-export.mdx index 9a5b970168..adcbe256dd 100644 --- a/docs/observability/ocsf-json-export.mdx +++ b/docs/observability/ocsf-json-export.mdx @@ -63,9 +63,13 @@ $env:OPENSHELL_OCSF_LOG_DIR = "D:\OpenShell\audit" openshell-gateway --drivers mxc --config gateway.toml ``` -The sink is available only when Windows selects the MXC driver. It rotates +This environment-configured sink is available on all platforms independently of +the selected compute driver. On Unix hosts its default directory is +`$XDG_STATE_HOME/openshell/logs` (or `$HOME/.local/state/openshell/logs`). It rotates daily and retains the three most recent files. Diagnostic `--log-level` values do not suppress records in this explicitly enabled audit sink. +Prefer the `gateway.toml` output above for configured retention and loss metrics; +enabling both sinks writes to both independently. ## Output Location diff --git a/sdk/go/openshell/v1/gateway/gateway.go b/sdk/go/openshell/v1/gateway/gateway.go index da7c9814b4..dde3545664 100644 --- a/sdk/go/openshell/v1/gateway/gateway.go +++ b/sdk/go/openshell/v1/gateway/gateway.go @@ -135,7 +135,7 @@ func ListGateways() ([]Info, error) { } } - sysNames, listErr := listGatewayDirs(systemConfigBase) + sysNames, listErr := listGatewayDirs(systemConfigDir()) if listErr != nil { return nil, listErr } diff --git a/sdk/go/openshell/v1/gateway/gateway_test.go b/sdk/go/openshell/v1/gateway/gateway_test.go index ec519c29a6..e22a0a7873 100644 --- a/sdk/go/openshell/v1/gateway/gateway_test.go +++ b/sdk/go/openshell/v1/gateway/gateway_test.go @@ -409,6 +409,7 @@ func TestLoadConfig_ActiveGateway(t *testing.T) { func TestListGateways_MultipleGateways(t *testing.T) { tmp := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv(systemGatewayDirEnv, tmp) for _, name := range []string{"prod", "staging", "dev"} { gwDir := filepath.Join(tmp, "openshell", "gateways", name) @@ -432,6 +433,7 @@ func TestListGateways_MultipleGateways(t *testing.T) { func TestListGateways_EmptyDirs(t *testing.T) { tmp := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv(systemGatewayDirEnv, tmp) gateways, err := ListGateways() require.NoError(t, err) @@ -441,6 +443,7 @@ func TestListGateways_EmptyDirs(t *testing.T) { func TestListGateways_ActiveStatus(t *testing.T) { tmp := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv(systemGatewayDirEnv, tmp) for _, name := range []string{"alpha", "beta"} { gwDir := filepath.Join(tmp, "openshell", "gateways", name) diff --git a/sdk/go/openshell/v1/gateway/paths.go b/sdk/go/openshell/v1/gateway/paths.go index fa0e1373c2..0fa9cb0092 100644 --- a/sdk/go/openshell/v1/gateway/paths.go +++ b/sdk/go/openshell/v1/gateway/paths.go @@ -24,6 +24,10 @@ const ( // systemConfigBase is the system-wide config directory. systemConfigBase = "/etc/openshell" + + // systemGatewayDirEnv overrides the system-wide config root. Keep this in + // sync with the Rust CLI so SDK discovery sees the same gateway set. + systemGatewayDirEnv = "OPENSHELL_SYSTEM_GATEWAY_DIR" ) // userConfigDir returns the user-specific configuration directory for @@ -49,7 +53,16 @@ func userConfigDir() (string, error) { // systemGatewayDir returns the system-wide gateway config directory. func systemGatewayDir() string { - return filepath.Join(systemConfigBase, gatewaySubdir) + return filepath.Join(systemConfigDir(), gatewaySubdir) +} + +// systemConfigDir returns the system-wide configuration root. Empty and +// relative overrides are ignored to match the CLI's fail-safe behavior. +func systemConfigDir() string { + if dir := os.Getenv(systemGatewayDirEnv); dir != "" && filepath.IsAbs(dir) { + return dir + } + return systemConfigBase } // resolveGatewayDir searches for a gateway directory by name, checking the diff --git a/sdk/go/openshell/v1/gateway/paths_test.go b/sdk/go/openshell/v1/gateway/paths_test.go index fff9a01d73..60a91569ea 100644 --- a/sdk/go/openshell/v1/gateway/paths_test.go +++ b/sdk/go/openshell/v1/gateway/paths_test.go @@ -39,6 +39,22 @@ func TestSystemGatewayDir(t *testing.T) { assert.Equal(t, filepath.FromSlash("/etc/openshell/gateways"), dir) } +func TestSystemGatewayDirOverride(t *testing.T) { + tmp := t.TempDir() + t.Setenv(systemGatewayDirEnv, tmp) + + assert.Equal(t, filepath.Join(tmp, "gateways"), systemGatewayDir()) +} + +func TestSystemGatewayDirIgnoresInvalidOverrides(t *testing.T) { + for _, override := range []string{"", "relative/path"} { + t.Run(override, func(t *testing.T) { + t.Setenv(systemGatewayDirEnv, override) + assert.Equal(t, filepath.FromSlash("/etc/openshell/gateways"), systemGatewayDir()) + }) + } +} + func TestResolveGatewayDir_UserDir(t *testing.T) { tmp := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmp) diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 10856eefe5..a9d1540431 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -273,9 +273,10 @@ Key flags: - `--gpu [COUNT]`: Request the driver's default GPU selection or a specific GPU count - `--cpu`, `--memory`: Set per-sandbox compute sizing. Docker/Podman apply limits; Kubernetes applies matching requests and limits. - `--driver-config-json`: Pass experimental driver-specific sandbox configuration -- `--template NAME`: Create from a named sandbox workload template. Conflicts with inline workload flags such as `--from`, `--gpu`, `--cpu`, `--memory`, `--env`, and `--driver-config-json`. +- `--template NAME`: Create from a named sandbox workload template. Conflicts with inline workload flags such as `--from`, `--gpu`, `--cpu`, `--memory`, `--env`, `--env-from`, and `--driver-config-json`. - `--label KEY=VALUE`: Add labels for later selection (repeatable) - `--env KEY=VALUE`: Set non-secret sandbox environment variables (repeatable); use `--provider` for credentials +- `--env-from KEY[=ENVVAR]`: Read a sandbox environment value from the CLI process environment without putting its value in arguments; use `--provider` to keep credentials hidden from the workload - `--tty`: Allocate a retained PTY for the canonical main process - `--restart-policy never|on-failure|always`: Select gateway-owned main-process restart behavior; `never` is the default - `--approval-mode manual|auto`: Control handling of agent-authored policy proposals; `manual` is the default 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"