From 34c67cd5e6e3f342d2daa03e6cecef34dc632299 Mon Sep 17 00:00:00 2001 From: Kris Hicks Date: Thu, 22 Oct 2026 16:36:28 -0700 Subject: [PATCH 1/5] fix(isolation): validate Unix socket paths the same way on every host Prior to this change, SandboxTransport::validate checked Unix socket paths with Path::is_absolute(). On Windows that requires a drive or UNC prefix, so a POSIX path such as /run/backend.sock was rejected as "invalid boundary control transport endpoint". The isolation backend registration work in #3898 added tests that register Unix endpoints in the core registration types, gateway config, gateway config preflight, and supervisor backend setup; all of them failed on the Windows MSVC runners. The Windows test job only runs on PRs labeled test:windows, so the break went unnoticed on main. Unix transport is refused at runtime on non-Unix hosts, so these paths are always POSIX paths regardless of where they are validated. Now, both the core registration validator and the sandbox backend protocol validator use a new openshell_core::paths::is_posix_absolute helper, which applies the POSIX rule (the path starts with '/') on every host. On Unix this matches is_absolute() exactly. Signed-off-by: Kris Hicks --- .../openshell-core/src/isolation_registration.rs | 3 ++- crates/openshell-core/src/paths.rs | 14 ++++++++++++++ .../src/boundary_protocol.rs | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/crates/openshell-core/src/isolation_registration.rs b/crates/openshell-core/src/isolation_registration.rs index 1633dbb742..8f820e302d 100644 --- a/crates/openshell-core/src/isolation_registration.rs +++ b/crates/openshell-core/src/isolation_registration.rs @@ -34,7 +34,8 @@ pub enum SandboxTransport { impl SandboxTransport { pub fn validate(&self) -> Result<(), &'static str> { let valid = match self { - Self::Unix { socket_path } => socket_path.is_absolute(), + // Unix transport only runs on Unix hosts, but validation may not. + Self::Unix { socket_path } => crate::paths::is_posix_absolute(socket_path), Self::Tcp { authority, addresses, diff --git a/crates/openshell-core/src/paths.rs b/crates/openshell-core/src/paths.rs index 79124738c1..0470ab6764 100644 --- a/crates/openshell-core/src/paths.rs +++ b/crates/openshell-core/src/paths.rs @@ -167,6 +167,11 @@ pub fn is_file_permissions_too_open(path: &Path) -> bool { std::fs::metadata(path).is_ok_and(|m| m.permissions().mode() & 0o077 != 0) } +/// Whether `path` is absolute under POSIX rules, whatever the host platform. +pub fn is_posix_absolute(path: &Path) -> bool { + path.to_str().is_some_and(|path| path.starts_with('/')) +} + /// Compatibility re-export; authored policy path normalization is owned by /// `openshell-policy-schema`. pub use openshell_policy_schema::normalize_path; @@ -175,6 +180,15 @@ pub use openshell_policy_schema::normalize_path; mod tests { use super::*; + #[test] + fn is_posix_absolute_ignores_host_path_syntax() { + assert!(is_posix_absolute(Path::new("/run/backend.sock"))); + assert!(is_posix_absolute(Path::new("/"))); + for path in ["", "relative", "./run", r"\run", r"C:\run", "C:/run"] { + assert!(!is_posix_absolute(Path::new(path)), "{path}"); + } + } + #[test] fn xdg_config_dir_respects_env() { // This test checks the logic — actual env var mutation is unsafe so diff --git a/crates/openshell-sandbox-backend/src/boundary_protocol.rs b/crates/openshell-sandbox-backend/src/boundary_protocol.rs index 8402a5e7a5..81325d0d22 100644 --- a/crates/openshell-sandbox-backend/src/boundary_protocol.rs +++ b/crates/openshell-sandbox-backend/src/boundary_protocol.rs @@ -283,7 +283,7 @@ impl SandboxTransport { pub fn validate(&self) -> Result<(), &'static str> { let valid = match self { Self::Unix { socket_path } | Self::RuntimeInitiatedUnix { socket_path } => { - socket_path.is_absolute() + openshell_core::paths::is_posix_absolute(socket_path) } Self::Tcp { authority, From 30f1a37c330c8a8a3f54f279803498e3b78a5c04 Mon Sep 17 00:00:00 2001 From: Kris Hicks Date: Thu, 22 Oct 2026 16:36:28 -0700 Subject: [PATCH 2/5] test(supervisor): run baseline write-back cases only on Unix hosts The startup policy write-refusal tests from #3785 rely on enrich_proto_baseline_paths adding the proxy baseline paths (/usr, /tmp, /dev/null, ...) to a policy, which triggers a write-back that the fake gateway then refuses. Enrichment only adds paths that exist on the host. Prior to this change, the tests assumed those paths exist, so on Windows enrichment added nothing, the precondition assertions failed, and the baseline case of permanent_codes_still_exit never saw a write to refuse. Now, the four baseline write-back tests are gated to Unix, and permanent_codes_still_exit checks the image policy upload case on every host and the baseline write-back case only on Unix. Signed-off-by: Kris Hicks --- crates/openshell-supervisor/src/lib.rs | 32 ++++++++++++++++---------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 46eb9e8839..a84c70a4a3 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -5620,6 +5620,8 @@ network_policies: .await; } + // Baseline enrichment adds host paths that only exist on Unix. + #[cfg(unix)] #[tokio::test(start_paused = true)] async fn startup_policy_write_refusal_baseline_write_back_waits_for_repair() { // The gateway policy has network rules but lacks the proxy baseline @@ -5749,6 +5751,8 @@ network_policies: .await; } + // Baseline enrichment adds host paths that only exist on Unix. + #[cfg(unix)] #[tokio::test(start_paused = true)] async fn startup_policy_write_refusal_baseline_repair_before_report_refetches() { let mut repaired = proto_tcp_policy_fixture(); @@ -5892,6 +5896,8 @@ network_policies: .await; } + // Baseline enrichment adds host paths that only exist on Unix. + #[cfg(unix)] #[tokio::test(start_paused = true)] async fn startup_policy_write_refusal_write_back_outlasts_the_attempt_limit() { // The same limit applies to a refused baseline-path write-back. @@ -5979,6 +5985,8 @@ network_policies: ); } + // Baseline enrichment adds host paths that only exist on Unix. + #[cfg(unix)] #[tokio::test(start_paused = true)] async fn startup_policy_write_refusal_global_policy_skips_baseline_write_back() { use openshell_core::proto::{ConfigurationAdmissionState, PolicySource}; @@ -6067,18 +6075,18 @@ network_policies: // Both writes are checked: the image policy upload to a gateway with // no policy, and the write-back of a stored policy that lacks the // proxy baseline paths. - let writes = [ - ( - "image policy upload", - unset_policy_snapshot(), - ImagePolicyDiscovery::Policy(Box::new(proto_tcp_policy_fixture())), - ), - ( - "baseline write-back", - settings_poll_result(Some(proto_tcp_policy_fixture()), 1, PolicySource::Sandbox), - ImagePolicyDiscovery::Missing, - ), - ]; + let image_upload = ( + "image policy upload", + unset_policy_snapshot(), + ImagePolicyDiscovery::Policy(Box::new(proto_tcp_policy_fixture())), + ); + let baseline_write_back = ( + "baseline write-back", + settings_poll_result(Some(proto_tcp_policy_fixture()), 1, PolicySource::Sandbox), + ImagePolicyDiscovery::Missing, + ); + // Baseline enrichment adds host paths that only exist on Unix. + let writes = std::iter::once(image_upload).chain(cfg!(unix).then_some(baseline_write_back)); for (write, initial, discovery) in writes { for code in [ tonic::Code::PermissionDenied, From 0f6ddbea830a6dc2ee9e82252b190b3f67b61e76 Mon Sep 17 00:00:00 2001 From: Kris Hicks Date: Thu, 22 Oct 2026 16:39:44 -0700 Subject: [PATCH 3/5] test(server): scale the SQLite concurrent-write test for Windows fsync sqlite_concurrent_writes_return_their_own_resource_version (#4408) runs 8 writers that each make 25 unconditional and 25 version-checked writes against one row of an on-disk WAL database with synchronous=FULL. Writers serialize on the SQLite write lock and wait out sqlx's default 5s busy_timeout. Prior to this change, the per-commit fsync cost on the Windows runners was high enough that a waiting writer starved past that timeout and failed with "database is locked" after about 7s. Now, each writer makes 5 writes on Windows and 25 elsewhere. The test still races every writer on the same row, which is what it checks. Signed-off-by: Kris Hicks --- crates/openshell-server/src/persistence/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/openshell-server/src/persistence/tests.rs b/crates/openshell-server/src/persistence/tests.rs index be86441245..3493b277dc 100644 --- a/crates/openshell-server/src/persistence/tests.rs +++ b/crates/openshell-server/src/persistence/tests.rs @@ -2379,7 +2379,8 @@ async fn sqlite_concurrent_writes_return_their_own_resource_version() { use std::sync::Arc; const WRITERS: u64 = 8; - const WRITES: u64 = 25; + // Slow Windows fsync can starve a writer past the SQLite busy timeout. + const WRITES: u64 = if cfg!(windows) { 5 } else { 25 }; let tmp = tempfile::tempdir().expect("tempdir"); let url = on_disk_store_url(&tmp.path().join("openshell.db")); From fa1ee608a86df0dc91146dca870dacaef5aca272 Mon Sep 17 00:00:00 2001 From: Kris Hicks Date: Thu, 22 Oct 2026 16:39:50 -0700 Subject: [PATCH 4/5] test(server): hold scope-mix locks for wall-clock time, not timer ticks random_scope_mix_never_deadlocks (#4147) runs 64 tasks of 50 random lock operations and holds each operation's locks for 0-2 ms, failing if the mix does not finish within 20s. Prior to this change, the hold was a tokio::time::sleep. Windows timers round a 1 ms sleep up to about 16 ms, and a quarter of the operations take the exclusive global lock, so the serialized holds alone took several seconds and the full mix exceeded the 20s deadline on the Windows runners. Now, occupy yields until a std::time::Instant deadline, so holds last 0-2 ms of wall-clock time on every platform and the deadline means the same thing everywhere. Signed-off-by: Kris Hicks --- crates/openshell-server/src/compute/mutation_guard.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/openshell-server/src/compute/mutation_guard.rs b/crates/openshell-server/src/compute/mutation_guard.rs index 0d137885a5..5b257774bc 100644 --- a/crates/openshell-server/src/compute/mutation_guard.rs +++ b/crates/openshell-server/src/compute/mutation_guard.rs @@ -1169,7 +1169,11 @@ mod tests { if op.is_distributed() { fleet.enter(op); } - tokio::time::sleep(hold).await; + // Windows timers round a 1 ms sleep up to ~16 ms, so yield instead. + let until = std::time::Instant::now() + hold; + while std::time::Instant::now() < until { + tokio::task::yield_now().await; + } if op.is_distributed() { fleet.leave(op); } From e7ce2a83ae37bc6ba56be6fd1f52ca7e7acef3d6 Mon Sep 17 00:00:00 2001 From: Kris Hicks Date: Fri, 23 Oct 2026 07:27:56 -0700 Subject: [PATCH 5/5] test(supervisor): run sandbox instance host tests only on Unix hosts The sandbox_instance_host tests added with per-instance supervisor state (#4701) admit hosted instances through SandboxInstanceHost::new, which prepares a private directory per instance. Hosted sandbox instances are Unix-only: on other hosts prepare_private_dir returns "hosted sandbox instances require a Unix host". Prior to this change, the test module still compiled and ran on Windows, so all three tests failed while creating their host. Now, the test module is gated to Unix, and the Unix-only permission check inside one test no longer needs its own cfg attribute. Signed-off-by: Kris Hicks --- crates/openshell-supervisor/src/sandbox_instance_host.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/openshell-supervisor/src/sandbox_instance_host.rs b/crates/openshell-supervisor/src/sandbox_instance_host.rs index 66aa30e57e..7465701460 100644 --- a/crates/openshell-supervisor/src/sandbox_instance_host.rs +++ b/crates/openshell-supervisor/src/sandbox_instance_host.rs @@ -745,7 +745,8 @@ pub(crate) fn prepare_private_dir(_path: &Path) -> Result<()> { )) } -#[cfg(test)] +// Hosted sandbox instances require a Unix host. +#[cfg(all(test, unix))] mod tests { use super::*; use http_body_util::BodyExt as _; @@ -1050,7 +1051,6 @@ mod tests { // Files that cannot be deleted keep the key reserved until a later // removal deletes them. Root ignores the permission this relies on. - #[cfg(unix)] if !nix::unistd::geteuid().is_root() { use std::os::unix::fs::PermissionsExt as _;