diff --git a/bazel/rules/testing/wine/BUILD.bazel b/bazel/rules/testing/wine/BUILD.bazel index 4bd9acf6d7..1f5d523a08 100644 --- a/bazel/rules/testing/wine/BUILD.bazel +++ b/bazel/rules/testing/wine/BUILD.bazel @@ -28,6 +28,20 @@ rust_library( ], ) +rust_binary( + name = "wine-test-exec", + testonly = True, + srcs = ["src/exec.rs"], + crate_name = "wine_test_exec", + crate_root = "src/exec.rs", + edition = "2024", + target_compatible_with = ["@platforms//os:linux"], + deps = [ + ":wine_test_support", + "@crates//:anyhow", + ], +) + rust_binary( name = "windows-smoke", testonly = True, diff --git a/bazel/rules/testing/wine/fixtures/windows_smoke.rs b/bazel/rules/testing/wine/fixtures/windows_smoke.rs index ffd98a2443..5703194e17 100644 --- a/bazel/rules/testing/wine/fixtures/windows_smoke.rs +++ b/bazel/rules/testing/wine/fixtures/windows_smoke.rs @@ -4,10 +4,15 @@ fn main() { println!("WINE_TEST_READY"); std::io::stdout().flush().expect("flush readiness marker"); - if std::env::args().any(|arg| arg == "--fail") { + let args = std::env::args().collect::>(); + if args.iter().any(|arg| arg == "--write-prefix-marker") { + std::fs::write(r"C:\shared-prefix-marker", b"shared prefix") + .expect("write shared-prefix marker"); + } + if args.iter().any(|arg| arg == "--fail") { std::process::exit(9); } - if std::env::args().any(|arg| arg == "--wait") { + if args.iter().any(|arg| arg == "--wait") { loop { std::thread::park(); } diff --git a/bazel/rules/testing/wine/src/exec.rs b/bazel/rules/testing/wine/src/exec.rs new file mode 100644 index 0000000000..8caa468c3e --- /dev/null +++ b/bazel/rules/testing/wine/src/exec.rs @@ -0,0 +1,17 @@ +use anyhow::Context; +use anyhow::Result; + +fn main() -> Result<()> { + let mut args = std::env::args_os().skip(1); + let executable = args + .next() + .context("usage: wine-test-exec [args...]")?; + let status = wine_test_support::ambient_wine_command(executable)? + .args(args) + .status() + .context("run Windows command in shared Wine test prefix")?; + if !status.success() { + std::process::exit(status.code().unwrap_or(1)); + } + Ok(()) +} diff --git a/bazel/rules/testing/wine/src/lib.rs b/bazel/rules/testing/wine/src/lib.rs index 3b99f5870b..5fb706e5e5 100644 --- a/bazel/rules/testing/wine/src/lib.rs +++ b/bazel/rules/testing/wine/src/lib.rs @@ -1,6 +1,7 @@ #[cfg(not(target_os = "linux"))] compile_error!("wine_test_support can only run on Linux"); +use std::ffi::OsStr; use std::ffi::OsString; use std::fs; use std::future::Future; @@ -18,6 +19,8 @@ use tokio::process::Child; use tokio::process::ChildStdout; use tokio::process::Command as TokioCommand; +const WINE_TEST_PREFIX_ENV_VAR: &str = "CODEX_WINE_TEST_PREFIX"; + /// Builds a command that runs a Windows executable in an isolated Wine prefix. pub struct WineTestCommand { executable: PathBuf, @@ -34,6 +37,16 @@ pub struct WineTestProcess { processes: Option, } +/// Ambient context for launching another Windows process in an active test prefix. +/// +/// Applying this context to a host command does not transfer ownership of the +/// prefix. The command must finish before the [`WineTestProcess`] that created +/// this context is shut down. +#[derive(Clone, Debug)] +pub struct WineTestCommandContext { + prefix: PathBuf, +} + struct WineProcesses { child: Child, cleanup_complete: bool, @@ -119,6 +132,16 @@ impl WineTestProcess { stdout } + /// Returns a context that a host subprocess can use to join this Wine prefix. + pub fn command_context(&self) -> WineTestCommandContext { + let Some(processes) = self.processes.as_ref() else { + panic!("Wine process guard is missing"); + }; + WineTestCommandContext { + prefix: processes.prefix.path().to_path_buf(), + } + } + /// Runs `future`, then asynchronously tears down Wine before returning. /// /// If both the scoped operation and teardown fail, the operation error is @@ -148,6 +171,26 @@ impl WineTestProcess { } } +impl WineTestCommandContext { + /// Exports this context to a host command and its descendants. + pub fn apply_to_command(&self, command: &mut TokioCommand) { + command.env(WINE_TEST_PREFIX_ENV_VAR, &self.prefix); + } +} + +/// Builds a Wine command that joins the prefix exported by [`WineTestCommandContext`]. +pub fn ambient_wine_command(executable: impl AsRef) -> Result { + let prefix = PathBuf::from( + std::env::var_os(WINE_TEST_PREFIX_ENV_VAR) + .with_context(|| format!("{WINE_TEST_PREFIX_ENV_VAR} must be set"))?, + ); + let runtime = WineRuntimePaths::from_runfiles()?; + let mut command = StdCommand::new(&runtime.wine); + configure_wine_environment(&mut command, &runtime, &prefix); + command.arg(executable); + Ok(command) +} + impl Drop for WineTestProcess { fn drop(&mut self) { // Panicking here starts unwinding, after which WineProcesses performs diff --git a/bazel/rules/testing/wine/src/lib_tests.rs b/bazel/rules/testing/wine/src/lib_tests.rs index e32d17399f..03cfeb92a9 100644 --- a/bazel/rules/testing/wine/src/lib_tests.rs +++ b/bazel/rules/testing/wine/src/lib_tests.rs @@ -148,6 +148,41 @@ async fn scope_returns_value_and_tears_down() -> Result<()> { Ok(()) } +#[tokio::test] +async fn exported_command_context_reuses_active_prefix() -> Result<()> { + let process = waiting_smoke_process().await?; + let prefix = prefix_path(&process); + let prefix_after_scope = prefix.clone(); + let marker = prefix.join("drive_c").join("shared-prefix-marker"); + let context = process.command_context(); + let wrapper = codex_utils_cargo_bin::cargo_bin("wine-test-exec")?; + let smoke = codex_utils_cargo_bin::cargo_bin("wine-smoke")?; + + process + .scope(async move { + let mut command = TokioCommand::new(wrapper); + context.apply_to_command(&mut command); + let output = command + .arg(smoke) + .arg("--write-prefix-marker") + .output() + .await + .context("run Windows process through exported Wine test context")?; + anyhow::ensure!( + output.status.success(), + "shared-prefix command failed: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim(), + ); + assert_eq!(fs::read(&marker)?, b"shared prefix"); + Ok(()) + }) + .await?; + + assert_prefix_removed(&prefix_after_scope); + Ok(()) +} + #[tokio::test] async fn scope_returns_body_error_and_tears_down() -> Result<()> { let process = waiting_smoke_process().await?; diff --git a/bazel/rules/testing/wine/wine.bzl b/bazel/rules/testing/wine/wine.bzl index 94b0d7a96e..4030e2c3c2 100644 --- a/bazel/rules/testing/wine/wine.bzl +++ b/bazel/rules/testing/wine/wine.bzl @@ -21,6 +21,8 @@ def wine_rust_test( * Each `host_binaries` and `windows_binaries` entry contributes `CARGO_BIN_EXE_` for its executable. * `CARGO_BIN_EXE_wine` and `CARGO_BIN_EXE_wineserver` identify Wine tools. + * `CARGO_BIN_EXE_wine-test-exec` identifies the host wrapper for running an + additional Windows command in an exported test prefix. * `CARGO_BIN_EXE_wine-runtime-marker` identifies a file whose parent is the Wine DLL directory to use as `WINEDLLPATH`. * `CARGO_BIN_EXE_pwsh` identifies the pinned PowerShell executable and diff --git a/bazel/rules/testing/wine/wine_runtime.bzl b/bazel/rules/testing/wine/wine_runtime.bzl index 9f47bdcff0..b69a73e846 100644 --- a/bazel/rules/testing/wine/wine_runtime.bzl +++ b/bazel/rules/testing/wine/wine_runtime.bzl @@ -5,6 +5,7 @@ _WINE_RUNTIME_BINARIES = { "pwsh-runtime-marker": "@powershell_windows_x86_64//:runtime_marker", "wine": "@wine_linux_x86_64//:wine", "wine-runtime-marker": "@wine_linux_x86_64//:runtime_marker", + "wine-test-exec": "//bazel/rules/testing/wine:wine-test-exec", "wineserver": "@wine_linux_x86_64//:wineserver", } diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index fb9a5fd041..628e9a40d1 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -4,11 +4,8 @@ use codex_config::types::ApprovalsReviewer; use codex_core::config::Constrained; use codex_exec_server::CopyOptions; use codex_exec_server::CreateDirectoryOptions; -use codex_exec_server::ExecParams; -use codex_exec_server::ExecProcessEvent; use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::LOCAL_ENVIRONMENT_ID; -use codex_exec_server::ProcessId; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::RemoveOptions; use codex_features::Feature; @@ -31,8 +28,6 @@ use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::ApiPathString; -use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; @@ -282,13 +277,13 @@ fn assert_normalized_path_rejected(error: &std::io::Error) { } } -fn remote_exec(script: &str) -> Result<()> { +fn remote_exec_docker(command: &str) -> Result<()> { let remote_env = get_remote_test_env().context("remote env should be configured")?; let container_name = remote_env .docker_container_name() .context("test requires direct access to the Docker container")?; let output = Command::new("docker") - .args(["exec", container_name, "sh", "-lc", script]) + .args(["exec", container_name, "sh", "-lc", command]) .output()?; assert!( output.status.success(), @@ -299,6 +294,36 @@ fn remote_exec(script: &str) -> Result<()> { Ok(()) } +fn remote_exec_wine_powershell(command: &str) -> Result<()> { + let wrapper = codex_utils_cargo_bin::cargo_bin("wine-test-exec")?; + let output = Command::new(wrapper) + .args([ + r"C:\Program Files\PowerShell\7\pwsh.exe", + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + command, + ]) + .output() + .context("run remote Wine command")?; + anyhow::ensure!( + output.status.success(), + "remote Wine command failed: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim(), + ); + Ok(()) +} + +fn remote_exec(linux_command: &str, windows_command: &str) -> Result<()> { + match core_test_support::test_environment() { + TestEnvironment::Docker { .. } => remote_exec_docker(linux_command), + TestEnvironment::WineExec => remote_exec_wine_powershell(windows_command), + TestEnvironment::Local => anyhow::bail!("test requires a remote environment"), + } +} + async fn exec_command_routing_output( test: &TestCodex, server: &wiremock::MockServer, @@ -1039,7 +1064,7 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn remote_test_env_sandboxed_read_rejects_symlink_parent_dotdot_escape() -> Result<()> { skip_if_no_network!(Ok(())); - let Some(remote_env) = get_remote_test_env() else { + let Some(_remote_env) = get_remote_test_env() else { return Ok(()); }; @@ -1055,87 +1080,27 @@ async fn remote_test_env_sandboxed_read_rejects_symlink_parent_dotdot_escape() - let outside_dir_uri = PathUri::from_abs_path(&outside_dir); let secret_path_uri = PathUri::from_abs_path(&secret_path); let symlink_path_uri = allowed_dir_uri.join("link")?; - file_system - .create_directory( - &allowed_dir_uri, - CreateDirectoryOptions { recursive: true }, - /*sandbox*/ None, - ) - .await?; - file_system - .create_directory( - &outside_dir_uri, - CreateDirectoryOptions { recursive: true }, - /*sandbox*/ None, - ) - .await?; - file_system - .write_file(&secret_path_uri, b"nope".to_vec(), /*sandbox*/ None) - .await?; - - match remote_env { - TestEnvironment::Docker { .. } => remote_exec(&format!( - "ln -s {} {}", - outside_dir.display(), - allowed_dir.join("link").display(), - ))?, - TestEnvironment::WineExec => { - let shell = test_env.environment().info().await?.shell; - let symlink_path = - ApiPathString::from_path_uri(&symlink_path_uri, PathConvention::Windows)?; - let outside_dir = - ApiPathString::from_path_uri(&outside_dir_uri, PathConvention::Windows)?; - let script = format!( - "$ErrorActionPreference = 'Stop'; New-Item -ItemType SymbolicLink -Path '{}' -Target '{}' | Out-Null", - symlink_path.as_str(), - outside_dir.as_str(), - ); - let process = test_env - .environment() - .get_exec_backend() - .start(ExecParams { - process_id: ProcessId::from("remote-env-create-symlink"), - argv: vec![ - shell.path, - "-NoLogo".to_string(), - "-NoProfile".to_string(), - "-NonInteractive".to_string(), - "-Command".to_string(), - script, - ], - cwd: PathUri::from_abs_path(test_env.cwd()), - env_policy: None, - env: Default::default(), - tty: false, - pipe_stdin: false, - arg0: None, - }) - .await? - .process; - let mut events = process.subscribe_events(); - let mut output = Vec::new(); - let mut exit_code = None; - loop { - match events.recv().await? { - ExecProcessEvent::Output(chunk) => { - output.extend(chunk.chunk.into_inner()); - } - ExecProcessEvent::Exited { - exit_code: code, .. - } => exit_code = Some(code), - ExecProcessEvent::Closed { .. } => break, - ExecProcessEvent::Failed(message) => anyhow::bail!(message), - } - } - assert_eq!( - exit_code, - Some(0), - "creating remote symlink failed: {}", - String::from_utf8_lossy(&output), - ); - } - TestEnvironment::Local => unreachable!("test requires a remote environment"), - } + let linux_setup_command = format!( + "rm -rf {root}; mkdir -p {allowed} {outside}; printf nope > {secret}; ln -s {outside} {allowed}/link", + root = root.display(), + allowed = allowed_dir.display(), + outside = outside_dir.display(), + secret = secret_path.display(), + ); + let windows_setup_command = format!( + r#"$ErrorActionPreference = 'Stop' +$root = ([Uri]'{root_uri}').LocalPath +$allowed = ([Uri]'{allowed_dir_uri}').LocalPath +$outside = ([Uri]'{outside_dir_uri}').LocalPath +$secret = ([Uri]'{secret_path_uri}').LocalPath +$link = ([Uri]'{symlink_path_uri}').LocalPath +Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue +New-Item -ItemType Directory -Path $allowed -Force | Out-Null +New-Item -ItemType Directory -Path $outside -Force | Out-Null +[System.IO.File]::WriteAllText($secret, 'nope') +New-Item -ItemType SymbolicLink -Path $link -Target $outside | Out-Null"#, + ); + remote_exec(&linux_setup_command, &windows_setup_command)?; let requested_path = allowed_dir_uri.join("link/../secret.txt")?; let sandbox = read_only_sandbox(allowed_dir.to_path_buf()); @@ -1145,16 +1110,13 @@ async fn remote_test_env_sandboxed_read_rejects_symlink_parent_dotdot_escape() - }; assert_normalized_path_rejected(&error); - file_system - .remove( - &root_uri, - RemoveOptions { - recursive: true, - force: true, - }, - /*sandbox*/ None, - ) - .await?; + let linux_cleanup_command = format!("rm -rf {}", root.display()); + let windows_cleanup_command = format!( + r#"$ErrorActionPreference = 'Stop' +$root = ([Uri]'{root_uri}').LocalPath +Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue"#, + ); + remote_exec(&linux_cleanup_command, &windows_cleanup_command)?; Ok(()) } @@ -1176,7 +1138,7 @@ async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> { let allowed_dir = root.join("allowed"); let outside_file = root.join("outside").join("keep.txt"); let symlink_path = allowed_dir.join("link"); - remote_exec(&format!( + remote_exec_docker(&format!( "rm -rf {root}; mkdir -p {allowed} {outside_parent}; printf outside > {outside}; ln -s {outside} {symlink}", root = root.display(), allowed = allowed_dir.display(), @@ -1248,7 +1210,7 @@ async fn remote_test_env_copy_preserves_symlink_source() -> Result<()> { let outside_file = root.join("outside").join("outside.txt"); let source_symlink = allowed_dir.join("link"); let copied_symlink = allowed_dir.join("copied-link"); - remote_exec(&format!( + remote_exec_docker(&format!( "rm -rf {root}; mkdir -p {allowed} {outside_parent}; printf outside > {outside}; ln -s {outside} {source}", root = root.display(), allowed = allowed_dir.display(), diff --git a/codex-rs/exec-server/testing/wine_exec_server.rs b/codex-rs/exec-server/testing/wine_exec_server.rs index 88857e785a..2aff4cd65f 100644 --- a/codex-rs/exec-server/testing/wine_exec_server.rs +++ b/codex-rs/exec-server/testing/wine_exec_server.rs @@ -7,6 +7,7 @@ use anyhow::Result; use tokio::io::AsyncBufReadExt; use tokio::io::BufReader; use wine_test_support::WineTestCommand; +use wine_test_support::WineTestCommandContext; /// Runs the Windows exec-server under Wine for the duration of a scoped operation. pub struct WineExecServer; @@ -17,12 +18,25 @@ impl WineExecServer { where F: FnOnce(String) -> Fut, Fut: Future>, + { + self.scope_with_command_context(|exec_server_url, _context| { + operation(exec_server_url) + }) + .await + } + + /// Starts the server and passes its URL and shared-prefix command context to `operation`. + pub async fn scope_with_command_context(self, operation: F) -> Result + where + F: FnOnce(String, WineTestCommandContext) -> Fut, + Fut: Future>, { let executable = codex_utils_cargo_bin::cargo_bin("wine-windows-exec-server")?; let mut exec_server = WineTestCommand::new(executable) .env("CODEX_HOME", r"C:\codex-home") .spawn()?; let stdout = exec_server.take_stdout(); + let command_context = exec_server.command_context(); exec_server .scope(async move { @@ -36,7 +50,7 @@ impl WineExecServer { break line; } }; - operation(exec_server_url).await + operation(exec_server_url, command_context).await }) .await } diff --git a/codex-rs/exec-server/testing/wine_remote_test_runner.rs b/codex-rs/exec-server/testing/wine_remote_test_runner.rs index 2889415675..bfd856b1e4 100644 --- a/codex-rs/exec-server/testing/wine_remote_test_runner.rs +++ b/codex-rs/exec-server/testing/wine_remote_test_runner.rs @@ -33,8 +33,9 @@ async fn main() -> Result<()> { } WineExecServer - .scope(|exec_server_url| async move { + .scope_with_command_context(|exec_server_url, command_context| async move { let mut command = Command::new(test_binary); + command_context.apply_to_command(&mut command); command .env(TEST_ENVIRONMENT_ENV_VAR, "wine-exec") .env(REMOTE_EXEC_SERVER_URL_ENV_VAR, exec_server_url)