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 e2179daf20..29f6416693 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5136,7 +5136,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 067f4ddcfd..5f20fc7f3c 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1452,7 +1452,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`) @@ -1597,6 +1597,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, @@ -3372,6 +3379,7 @@ async fn run_async() -> Result<()> { no_auto_providers, labels, envs, + env_from, no_credential_warnings, approval_mode, output, @@ -3409,7 +3417,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 49f6f05d81..807a79cb72 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, @@ -6911,12 +6911,12 @@ mod tests { ForwardTcpConnectionError, PolicyGetView, ProvisioningStep, build_sandbox_resource_limits, format_endpoint, format_log_line, forward_requires_session_token, 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_status_error, service_url_for_gateway, - workspace_member_to_json, + parse_driver_config_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_status_error, + service_url_for_gateway, workspace_member_to_json, }; use openshell_core::proto::TcpForwardFrame; @@ -7181,6 +7181,57 @@ mod tests { }); } + #[test] + fn parse_env_from_pairs_reads_named_and_same_name_environment_variables() { + Environment::new() + .set("NAV_PARSE_ENV_FROM_NAMED", "named-value") + .set("NAV_PARSE_ENV_FROM_SAME", "same-name-value") + .run(|| { + 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() { + Environment::new() + .remove("NAV_PARSE_ENV_FROM_MISSING") + .set("NAV_PARSE_ENV_FROM_PRESENT", "value") + .run(|| { + 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() { Environment::new() diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 44ddc5ebe7..e9584cd6e9 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -3333,6 +3333,16 @@ async fn run_cli_sandbox_create_with_xdg( xdg_dir: &TempDir, name: &str, extra_args: &[&str], +) -> 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_")) { @@ -3351,6 +3361,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") @@ -3488,6 +3499,47 @@ async fn sandbox_create_upload_warns_and_reaches_ssh_outside_git_repository() { .await; } +async fn run_cli_sandbox_create_with_env( + server: &TestServer, + name: &str, + extra_args: &[&str], + environment: &[(&str, &str)], +) -> std::process::Output { + let xdg_dir = helpers::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 9c079c73f8..d71f34b7c6 100644 --- a/crates/openshell-core/src/provider_credentials.rs +++ b/crates/openshell-core/src/provider_credentials.rs @@ -442,6 +442,18 @@ impl ProviderCredentialState { .requires_stable_placeholders } + /// 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-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 d2bd213e95..f7a7a43c3c 100644 --- a/crates/openshell-prover/src/containment.rs +++ b/crates/openshell-prover/src/containment.rs @@ -2249,6 +2249,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..1751ce5ab5 --- /dev/null +++ b/crates/openshell-sandbox-backend/src/audit.rs @@ -0,0 +1,71 @@ +// 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, + "socket_loopback_confinement": 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 13ee0fad34..5ffb83ac8e 100644 --- a/crates/openshell-sandbox-backend/src/lib.rs +++ b/crates/openshell-sandbox-backend/src/lib.rs @@ -10,12 +10,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 77dab9368b..b20ca44f3b 100644 --- a/crates/openshell-sandbox-backend/src/runtime.rs +++ b/crates/openshell-sandbox-backend/src/runtime.rs @@ -64,9 +64,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, @@ -78,7 +90,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( @@ -93,11 +114,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] @@ -121,10 +163,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, @@ -141,6 +182,7 @@ impl IsolationBackend for OpenShellRuntimeBackend { )); } Ok(Box::new(RemoteBound { + audit_validator: self.audit_validator.clone(), client: client.clone(), agent: sandbox.agent, policy: sandbox.policy, @@ -286,6 +328,7 @@ fn validate_control_port(port: u32) -> Result<(), BackendError> { } struct RemoteBound { + audit_validator: Arc, client: Arc, agent: AgentSpec, policy: openshell_core::policy::SandboxPolicy, @@ -326,17 +369,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(); @@ -1083,6 +1119,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, @@ -1246,6 +1283,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, @@ -1867,6 +1905,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 = @@ -1880,6 +1919,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 @@ -1887,7 +1927,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())) @@ -1908,10 +1948,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, @@ -1919,41 +1960,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 = @@ -2927,6 +2980,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, @@ -3123,6 +3177,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 0b75a45f99..da99518833 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -3,9 +3,12 @@ //! Capability-free in-workload sandbox boundary. +#[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")] @@ -14,16 +17,19 @@ pub(crate) mod delegated; pub mod linux; #[cfg(target_os = "linux")] pub mod main_session; +#[cfg(target_os = "linux")] pub mod managed_children; #[cfg(target_os = "linux")] mod network_broker; #[cfg(all(target_os = "linux", feature = "perf-harness"))] pub mod perf; -#[cfg(unix)] +#[cfg(target_os = "linux")] 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 3890d3a7a1..86ef2bb4f5 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"; @@ -1535,7 +1535,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 _; @@ -1592,7 +1592,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, @@ -1610,7 +1610,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 _}; @@ -1647,10 +1647,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 _}; @@ -1703,6 +1705,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 _; @@ -1747,6 +1750,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 = @@ -1813,7 +1823,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 715f3ef784..3449bd6d76 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -45,8 +45,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, SetResourceVersion}; @@ -79,6 +80,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; @@ -282,6 +300,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 +/// scoped mutation 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 { @@ -319,6 +347,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, @@ -1030,6 +1059,20 @@ impl ComputeRuntime { } pub async fn validate_sandbox_create(&self, sandbox: &Sandbox) -> Result<(), Status> { + self.validate_sandbox_create_with_runtime_inputs( + sandbox, + &SandboxCreateRuntimeInputs::default(), + false, + ) + .await + } + + pub(crate) async fn validate_sandbox_create_with_runtime_inputs( + &self, + sandbox: &Sandbox, + runtime_inputs: &SandboxCreateRuntimeInputs, + await_main_process_attachment: bool, + ) -> Result<(), Status> { self.validate_caller_driver_config( sandbox .spec @@ -1038,6 +1081,14 @@ impl ComputeRuntime { )?; let mut driver_sandbox = driver_sandbox_from_public(sandbox, &self.driver_info.name) .map_err(|status| *status)?; + if let Some(spec) = driver_sandbox.spec.as_mut() { + spec.await_main_process_attachment = await_main_process_attachment; + } + 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) { @@ -1131,11 +1182,60 @@ impl ComputeRuntime { await_main_process_attachment: bool, lifecycle_guard: SandboxLifecycleGuard, mutation_guard: MutationGuard, + ) -> 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, + mutation_guard, + )) + .await + } + + #[cfg(test)] + 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, mutation_guard) = self + .sandbox_create_guards(sandbox.object_workspace(), 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, + mutation_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, + mutation_guard: MutationGuard, ) -> 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()), )?; @@ -1159,6 +1259,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()); } @@ -1206,7 +1311,7 @@ impl ComputeRuntime { // cancelled. Keep the creation guard until its public identity and // protected launch bundle have been committed. let prepared = async { - if let Some(encoded) = launch_authentication { + if let Some(encoded) = runtime_inputs.launch_authentication { let authentication = serde_json::from_slice(&encoded) .map_err(|_| Status::internal("invalid sandbox launch authentication"))?; let encoded = self @@ -2456,8 +2561,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( @@ -2466,6 +2575,56 @@ impl ComputeRuntime { name: &str, allow_missing: bool, ) -> Result { + self.delete_sandbox_with_options( + workspace, + name, + allow_missing, + SandboxDeletePreconditions::default(), + ) + .await + } + + #[cfg(test)] + 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. @@ -2483,11 +2642,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( @@ -2495,16 +2663,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 sandbox_guard = self.lock_sandbox_for_lifecycle(&delete_guard).await; @@ -2518,7 +2690,13 @@ impl ComputeRuntime { let request_span = tracing::Span::current(); tokio::spawn( async move { - Box::pin(runtime.delete_sandbox_inner(target, delete_guard, sandbox_guard)).await + Box::pin(runtime.delete_sandbox_inner( + target, + preconditions, + delete_guard, + sandbox_guard, + )) + .await } .instrument(request_span), ) @@ -2533,6 +2711,7 @@ impl ComputeRuntime { async fn delete_sandbox_inner( &self, target: SandboxDeleteTarget, + preconditions: SandboxDeletePreconditions, delete_guard: SandboxLifecycleGuard, guard: LocalMutationGuard, ) -> Result { @@ -2556,6 +2735,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 @@ -7413,6 +7609,7 @@ 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, @@ -7775,7 +7972,10 @@ mod tests { workspace_rpcs_unimplemented: bool, omit_protocol_metadata: bool, requires_launch_authentication: bool, + validate_create_calls: AtomicUsize, + validated_sandboxes: TestMutex>, create_calls: AtomicUsize, + created_sandboxes: TestMutex>, } #[tonic::async_trait] @@ -7831,8 +8031,13 @@ mod tests { async fn validate_sandbox_create( &self, - _request: Request, + request: Request, ) -> Result, Status> { + self.validate_create_calls.fetch_add(1, Ordering::Relaxed); + self.validated_sandboxes + .lock() + .expect("validated sandbox observation lock poisoned") + .push(request.into_inner().sandbox.expect("validation sandbox")); Ok(tonic::Response::new(ValidateSandboxCreateResponse {})) } @@ -7882,9 +8087,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())) } @@ -8503,6 +8714,124 @@ 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 = test_launch_authentication(&sandbox); + 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" + ); + } + + #[tokio::test] + async fn preflight_and_create_receive_the_same_main_attachment_intent() { + for attachment_expected in [false, true] { + let driver = Arc::new(TestDriver::default()); + let runtime = test_runtime(driver.clone()).await; + let mut sandbox = + sandbox_record("sb-attachment", "attachment", SandboxPhase::Provisioning); + sandbox.spec = Some(SandboxSpec { + command: vec!["echo".into(), "ok".into()], + ..Default::default() + }); + let inputs = SandboxCreateRuntimeInputs::default(); + runtime + .validate_sandbox_create_with_runtime_inputs(&sandbox, &inputs, attachment_expected) + .await + .expect("preflight"); + runtime + .create_sandbox_with_runtime_inputs(sandbox, None, attachment_expected, inputs) + .await + .expect("create"); + let validated = driver.validated_sandboxes.lock().unwrap(); + let created = driver.created_sandboxes.lock().unwrap(); + assert_eq!(validated.len(), 1); + assert_eq!(created.len(), 1); + assert_eq!( + validated[0] + .spec + .as_ref() + .unwrap() + .await_main_process_attachment, + attachment_expected + ); + assert_eq!( + created[0] + .spec + .as_ref() + .unwrap() + .await_main_process_attachment, + attachment_expected + ); + } + } + async fn test_runtime_with_gateway_managed_lifecycle( driver: SharedComputeDriver, driver_name: &str, @@ -10885,7 +11214,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, }); @@ -13208,6 +13537,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 4bf28b9dfa..99fb1dde79 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -3568,6 +3568,50 @@ 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, @@ -3577,15 +3621,11 @@ pub(super) async fn handle_get_sandbox_provider_environment( let supports_stable_placeholder_environment_keys = request .get_ref() .supports_stable_placeholder_environment_keys; - 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, @@ -7177,17 +7217,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 { @@ -7198,12 +7238,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(), @@ -8695,6 +8729,18 @@ mod tests { policy } + #[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; @@ -24544,6 +24590,23 @@ 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, + supports_stable_placeholder_environment_keys: 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 7c2606cad4..8127760bce 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -79,6 +79,11 @@ pub(super) struct ProviderEnvironment { pub static_credential_keys: HashSet, pub stable_placeholder_environment_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. @@ -1136,6 +1141,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(); @@ -1273,6 +1279,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); @@ -1385,6 +1392,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 @@ -1464,6 +1487,7 @@ pub(super) async fn resolve_provider_environment_from_records_with_policy_bindin static_credential_keys, stable_placeholder_environment_keys, files, + expired_static_keys, }) } @@ -12639,6 +12663,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 346634711b..463176fb65 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -14,7 +14,7 @@ use crate::auth::workspace_authz::{ AuthorizedWorkspaceScope, MinWorkspaceRole, authorize_list_workspace_selector, authorize_sandbox_workspace, authorize_workspace, }; -use crate::compute::MutationScope; +use crate::compute::{MutationScope, SandboxDeletePreconditions}; use crate::pagination::Pagination; use crate::persistence::{ ObjectLabels, ObjectListQuery, ObjectType, WriteCondition, generate_name, @@ -159,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. @@ -639,12 +670,21 @@ async fn handle_create_sandbox_inner( ) .await?; + // Provider and policy reads must stay under the create mutation guard so + // their revisions cannot change before the sandbox's launch is committed. + 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_main_process_attachment, + ) .await .map_err(|status| { warn!(error = %status, "Rejecting sandbox create request"); @@ -674,12 +714,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, mutation_guard, )) @@ -1673,10 +1714,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); @@ -2637,8 +2686,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) = @@ -2845,13 +2894,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(); @@ -5817,6 +5868,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 c2c4ce5e75..1390f61a11 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -1250,6 +1250,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. /// @@ -1651,8 +1664,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) => { @@ -2120,6 +2145,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>, 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 @@ -279,6 +280,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 { @@ -501,6 +507,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( @@ -524,6 +531,7 @@ impl ProxyHandle { dtx, atx, endpoint_observations, + required_authorization, ) .await { @@ -1535,7 +1543,6 @@ fn build_accept_error_event( .message(message) .build() } - fn classify_accept_error( err: &std::io::Error, consecutive_resource_errors: &mut u32, @@ -1543,7 +1550,6 @@ fn classify_accept_error( ) -> AcceptAction { #[cfg(not(unix))] let _ = (err, &mut *consecutive_resource_errors); - #[cfg(unix)] if matches!( err.raw_os_error(), @@ -2295,6 +2301,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. @@ -2347,6 +2387,7 @@ async fn handle_tcp_connection( denial_tx, activity_tx, endpoint_observation_tx, + None, )) .await } @@ -2418,6 +2459,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. @@ -2520,6 +2562,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(); @@ -6989,6 +7045,25 @@ 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 crate::test_support::{executable_path, policy_with_native_executable_paths}; use openshell_core::proposals::AgentProposals; @@ -6998,9 +7073,15 @@ mod tests { 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_engine(rego: &str, source: &str) -> Result { + OpaEngine::from_strings(rego, &policy_with_native_executable_paths(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"), &policy_with_native_executable_paths( r#" @@ -7059,7 +7140,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"), &policy_with_native_executable_paths( r#" @@ -7118,7 +7199,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"), &policy_with_native_executable_paths( r#" @@ -7229,7 +7310,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"), &policy_with_native_executable_paths( r#" @@ -7282,7 +7363,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"), &policy_with_native_executable_paths( r#" @@ -7588,6 +7669,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } None, None, None, + None, )) .await .unwrap(); @@ -7696,7 +7778,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_with_native_executable_paths(POLICY_DNS_OPEN_POLICY), ) @@ -7736,7 +7818,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_with_native_executable_paths(POLICY_DNS_OPEN_POLICY), ) @@ -7779,7 +7861,7 @@ process: { run_as_user: sandbox, run_as_group: sandbox } #[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_with_native_executable_paths(POLICY_DNS_OPEN_POLICY), ) @@ -7845,7 +7927,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_with_native_executable_paths(POLICY_DNS_OPEN_POLICY), ) @@ -7939,7 +8021,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", ) @@ -8008,6 +8090,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") @@ -8058,7 +8141,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"), &policy_with_native_executable_paths(&format!( r#" @@ -8136,6 +8219,7 @@ network_policies: None, None, None, + None, )); let request = tokio::time::timeout(std::time::Duration::from_secs(5), request_rx) .await @@ -8264,7 +8348,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"), &policy_with_native_executable_paths( r#" @@ -8746,6 +8830,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 77ce2a9d6f..4d506f79d5 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 3bc4638f07..9f61c6e3e2 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() } @@ -91,78 +93,32 @@ 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 host_key = host_key.ok_or_else(|| miette::miette!("sandbox SSH host key is missing"))?; - - 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), - host_key, - ) - .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" - )); - } - } + 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(), + host_key, + ) + .await?; + // 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(), @@ -171,24 +127,95 @@ 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, + host_key: Option, +) -> Result>> { + let Some(socket_path) = socket_path else { + return Ok(None); + }; + let host_key = host_key.ok_or_else(|| miette::miette!("sandbox SSH host key is missing"))?; + 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), + host_key, + ) + .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, + _host_key: Option, +) -> 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, @@ -241,6 +268,146 @@ 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, + 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, + 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 96598a8cd4..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 rustix::termios::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,59 +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 winsize = rustix::termios::tcgetwinsize(master).expect("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. @@ -310,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 }), @@ -368,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) { @@ -643,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( @@ -651,36 +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), - }; - let _ = rustix::termios::tcsetwinsize(master, 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()) } @@ -695,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::*; @@ -780,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]); @@ -951,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 905a5f0a38..58b3941df1 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -1057,15 +1057,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"); } @@ -1538,7 +1537,6 @@ fn direct_tcpip_target( )] mod tests { use super::*; - use std::io::Write as _; pub(super) struct AcceptAnyServerKey; @@ -2119,14 +2117,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 07ff3da1d1..898af8672d 100644 --- a/crates/openshell-supervisor-process/src/supervisor_session.rs +++ b/crates/openshell-supervisor-process/src/supervisor_session.rs @@ -638,7 +638,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>, @@ -967,22 +967,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)) } } } @@ -1090,10 +1100,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( @@ -1120,6 +1128,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 4484745f65..d846507991 100644 --- a/crates/openshell-supervisor/README.md +++ b/crates/openshell-supervisor/README.md @@ -16,6 +16,12 @@ The client receives the supervisor's live provider state, bearer-token slot, and 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. + ## Console logging Diagnostics and OCSF shorthand share a bounded, nonblocking stderr writer. With `ocsf_json_enabled=true`, the same writer also receives timestamped `OCSF-JSON` records. Each formatter submits a complete line in one write so concurrent producers cannot interleave records in the queue. The 1,024-line queue drops new lines when full rather than waiting for stderr. @@ -34,3 +40,23 @@ protects both original pipe endpoints with close-on-exec and lets `Stdio` transf the read endpoint to the child's stdin. The supervisor uses the standard stdin reader and does not adopt a numeric raw descriptor. This mode reserves stdin for liveness; sandbox workload input travels over the boundary protocol. + +## 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 52bf73e150..3fbabe0cc1 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"; @@ -185,17 +186,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, + }), + }) } } @@ -352,6 +357,18 @@ impl NetworkMediationSource for TestIo { #[tonic::async_trait] impl BoundaryProcess for TestIo { + async fn attach( + &self, + ) -> std::result::Result + { + 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 { Ok(BoundaryExitStatus::Exited(0)) } @@ -467,7 +484,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( @@ -674,7 +692,7 @@ async fn shared_startup_uses_selected_backend_through_readiness_and_shutdown() { .observed .require_networking .store(true, Ordering::SeqCst); - let startup = || { + let startup = |await_main_process_attachment| { Box::pin(crate::run_sandbox_with_backend( &setup, crate::SandboxRunConfig { @@ -682,7 +700,7 @@ async fn shared_startup_uses_selected_backend_through_readiness_and_shutdown() { workdir: Some("/test-workspace".into()), timeout_secs: 15, interactive: false, - await_main_process_attachment: false, + await_main_process_attachment, sandbox_id: Some(setup.sandbox_id.clone()), sandbox: None, openshell_endpoint: None, @@ -702,11 +720,18 @@ async fn shared_startup_uses_selected_backend_through_readiness_and_shutdown() { )) }; + let error = startup(true) + .await + .expect_err("foreground attachment requires an adapter"); + assert!(error.to_string().contains("foreground attachment")); + assert!(setup.observed.events().is_empty()); + assert_eq!(setup.observed.starts.load(Ordering::SeqCst), 0); + setup .observed .discovery_unavailable .store(true, Ordering::SeqCst); - let error = tokio::time::timeout(Duration::from_secs(5), startup()) + let error = tokio::time::timeout(Duration::from_secs(5), startup(false)) .await .expect("discovery failure must end startup") .expect_err("discovery failure must reject startup"); @@ -728,7 +753,7 @@ async fn shared_startup_uses_selected_backend_through_readiness_and_shutdown() { .observed .deny_confirmation .store(true, Ordering::SeqCst); - assert!(startup().await.is_err()); + assert!(startup(false).await.is_err()); assert_eq!( setup.observed.events(), [ @@ -775,7 +800,7 @@ async fn shared_startup_uses_selected_backend_through_readiness_and_shutdown() { nix::sys::signal::kill(nix::unistd::Pid::this(), nix::sys::signal::Signal::SIGTERM) .unwrap(); }; - let (result, ()) = tokio::join!(startup(), observe_and_shutdown); + let (result, ()) = tokio::join!(startup(false), observe_and_shutdown); assert_eq!(result.unwrap(), 0); assert_eq!( setup.observed.events(), diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 71cfebe63a..b09c91842f 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -121,21 +121,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) @@ -147,7 +165,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. } } } @@ -174,6 +195,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), } @@ -190,9 +212,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()) @@ -302,6 +322,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(); @@ -593,6 +620,7 @@ pub async fn run_network_proxy( #[cfg(target_os = "linux")] None, None, + None, ) .await?; @@ -739,6 +767,13 @@ async fn run_sandbox_with_backend( admitted_isolation_backend, main_exit_marker, } = config; + // The current client attachment adapter is SSH; retaining a process's I/O + // internally does not make it reachable by a foreground client. + if await_main_process_attachment && (!cfg!(unix) || ssh_socket_path.is_none()) { + return Err(miette::miette!( + "foreground attachment to the main process requires a supported SSH access adapter; create the sandbox with --detach" + )); + } // An empty command is the versioned scratch-sandbox sentinel. The // external supervisor cannot inspect the workload filesystem, so preserve // it for openshell-sandbox to resolve against the agent image. @@ -784,6 +819,7 @@ async fn run_sandbox_with_backend( &auth_bundle, )?; let sandbox_bearer = openshell_core::grpc_client::install_supervisor_auth_bundle(&auth_bundle)?; + #[cfg(unix)] let ssh_host_key = if ssh_socket_path.is_some() { Some(openshell_supervisor_process::ssh::parse_host_key( auth_bundle.ssh_host_private_key.as_ref(), @@ -791,6 +827,15 @@ async fn run_sandbox_with_backend( } else { None }; + #[cfg(not(unix))] + let ssh_host_key = { + if ssh_socket_path.is_some() { + return Err(miette::miette!( + "SSH access sockets are unsupported on this host" + )); + } + None + }; // Startup joins the trace that created the sandbox when the driver passes // one, and ends once the access plane is up. let startup = tracing::info_span!( @@ -940,7 +985,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(), @@ -1024,7 +1069,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; @@ -1063,7 +1110,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?, ); @@ -1451,7 +1499,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`. @@ -5221,6 +5273,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(); @@ -5234,6 +5287,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(); @@ -5300,12 +5354,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; } }) @@ -5314,7 +5375,7 @@ mod tests { session_tx.send_replace(true); timeout(Duration::from_secs(1), async { - while !connects() { + while !connects().await { tokio::task::yield_now().await; } }) @@ -5323,7 +5384,7 @@ mod tests { drop(readiness); timeout(Duration::from_secs(1), async { - while connects() { + while connects().await { tokio::task::yield_now().await; } }) @@ -5331,7 +5392,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 45343da980..6f72fb9786 100644 --- a/crates/openshell-supervisor/src/main.rs +++ b/crates/openshell-supervisor/src/main.rs @@ -173,14 +173,16 @@ fn arm_parent_liveness(enabled: bool) -> 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, }) } @@ -295,8 +297,10 @@ fn main() -> Result<()> { validate_role_arguments(&args)?; arm_parent_liveness(args.parent_liveness_stdin)?; 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 @@ -653,7 +657,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] @@ -715,6 +726,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 e634012037..30a75f7f6a 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -981,6 +981,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 c3f6f86dcb..f10cccb561 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -33,6 +33,9 @@ attaching: openshell sandbox create --name worker --detach -- ./worker ``` +Drivers that do not support foreground attachment reject the request before +provisioning. Use `--detach` with those drivers. + Use `--restart-policy on-failure` to replace the runtime after a nonzero main process exit, or `--restart-policy always` to replace it after every exit: @@ -393,6 +396,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 7e19d9d812..61738d08c2 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 76c7da014e..75d2de5057 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -303,9 +303,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 @@ -316,6 +317,8 @@ Key flags: - `--forward [BIND_ADDRESS:]PORT`: Forward a local port and keep the sandbox alive - `--editor vscode|cursor`: Open a remote editor after creation and keep the sandbox alive +If the driver rejects foreground attachment during create validation, use `--detach`. + `--detach` adds no attachment grace period. When the canonical process exits, its terminal phase is reported immediately. A foreground create declares one expected main-process SSH attachment; cleanup finalizes after that connection 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"