From f2c6707fd10a22db61e0834d571e4c6c3052e99e Mon Sep 17 00:00:00 2001 From: Dror Meirovich Date: Sun, 8 Nov 2026 19:32:02 +0200 Subject: [PATCH 1/3] fix(windows): fix native runtime compatibility and enable workspace tests Fix shell discovery, hook quoting, native path handling and OAuth randomness on Windows, make the test fixtures portable, and run the full workspace test suite in the Windows CI job. --- .github/workflows/rust-ci.yml | 7 +- rust/Cargo.lock | 1 + rust/crates/api/src/client.rs | 12 +- rust/crates/api/src/lib.rs | 10 ++ rust/crates/api/src/prompt_cache.rs | 9 +- rust/crates/api/src/providers/anthropic.rs | 9 +- rust/crates/api/src/providers/mod.rs | 13 +- .../crates/api/src/providers/openai_compat.rs | 9 +- rust/crates/api/tests/proxy_integration.rs | 30 +++-- rust/crates/plugins/src/hooks.rs | 96 ++++++++++----- rust/crates/plugins/src/lib.rs | 31 ++--- rust/crates/runtime/Cargo.toml | 1 + rust/crates/runtime/src/bash.rs | 39 +++++- rust/crates/runtime/src/config.rs | 4 +- rust/crates/runtime/src/conversation.rs | 28 ++--- rust/crates/runtime/src/file_ops.rs | 18 ++- rust/crates/runtime/src/hooks.rs | 79 ++++++------ rust/crates/runtime/src/lib.rs | 14 +++ rust/crates/runtime/src/mcp_stdio.rs | 54 ++++---- rust/crates/runtime/src/mcp_tool_bridge.rs | 9 +- rust/crates/runtime/src/oauth.rs | 6 +- rust/crates/runtime/src/session.rs | 6 + rust/crates/rusty-claude-cli/src/main.rs | 53 +++++--- .../rusty-claude-cli/tests/compact_output.rs | 8 +- .../tests/compact_repl_panic.rs | 36 ++---- .../tests/mock_parity_harness.rs | 107 ++++++++++------ .../tests/output_format_contract.rs | 31 ++++- .../tests/parallel_tool_result_pairing.rs | 4 +- .../rusty-claude-cli/tests/support/mod.rs | 27 ++++ .../tests/support/repl_console.py | 92 ++++++++++++++ rust/crates/tools/src/lib.rs | 115 ++++++++++++------ 31 files changed, 630 insertions(+), 328 deletions(-) create mode 100644 rust/crates/rusty-claude-cli/tests/support/mod.rs create mode 100644 rust/crates/rusty-claude-cli/tests/support/repl_console.py diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index c6d4e2f89e..c4ba3ff866 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -126,7 +126,7 @@ jobs: run: cargo clippy --workspace windows-smoke: - name: windows PowerShell smoke + name: Windows tests and PowerShell smoke runs-on: windows-latest defaults: run: @@ -138,6 +138,11 @@ jobs: - uses: Swatinem/rust-cache@49a0bdc70d2e1b713ca9e2869b211fcce03d3c1c with: workspaces: rust -> target + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 + with: + python-version: "3.x" + - name: Run Windows workspace tests + run: cargo test --workspace --locked --no-fail-fast - name: Build CLI for Windows smoke run: cargo build -p rusty-claude-cli - name: Smoke local commands without live credentials diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 2c96cd44e9..ab9412a0e3 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2138,6 +2138,7 @@ dependencies = [ name = "runtime" version = "0.1.11" dependencies = [ + "getrandom 0.2.17", "glob", "plugins", "regex", diff --git a/rust/crates/api/src/client.rs b/rust/crates/api/src/client.rs index 57ea4ede05..3688179b6a 100644 --- a/rust/crates/api/src/client.rs +++ b/rust/crates/api/src/client.rs @@ -281,21 +281,11 @@ pub fn read_xai_base_url() -> String { #[cfg(test)] mod tests { - use std::sync::{Mutex, OnceLock}; + use crate::test_env_lock as env_lock; use super::{ProviderClient, ProviderConfig, ProviderConfigKind}; use crate::providers::{detect_provider_kind, resolve_model_alias, ProviderKind}; - /// Serializes every test in this module that mutates process-wide - /// environment variables so concurrent test threads cannot observe - /// each other's partially-applied state. - fn env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - } - #[test] fn resolves_existing_and_grok_aliases() { assert_eq!(resolve_model_alias("opus"), "claude-opus-4-7"); diff --git a/rust/crates/api/src/lib.rs b/rust/crates/api/src/lib.rs index e03d78ee12..aa59bdb563 100644 --- a/rust/crates/api/src/lib.rs +++ b/rust/crates/api/src/lib.rs @@ -46,3 +46,13 @@ pub use telemetry::{ MemoryTelemetrySink, SessionTraceRecord, SessionTracer, TelemetryEvent, TelemetrySink, DEFAULT_ANTHROPIC_VERSION, }; + +// Environment variables are shared by every test module in this process. +// Module-local locks cannot protect provider tests from each other. +#[cfg(test)] +pub(crate) fn test_env_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::OnceLock> = std::sync::OnceLock::new(); + LOCK.get_or_init(|| std::sync::Mutex::new(())) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} diff --git a/rust/crates/api/src/prompt_cache.rs b/rust/crates/api/src/prompt_cache.rs index 0ee8663cc7..8fe6028011 100644 --- a/rust/crates/api/src/prompt_cache.rs +++ b/rust/crates/api/src/prompt_cache.rs @@ -500,7 +500,7 @@ fn stable_hash_bytes(bytes: &[u8]) -> u64 { #[cfg(test)] mod tests { - use std::sync::{Mutex, OnceLock}; + use crate::test_env_lock; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use super::{ @@ -509,13 +509,6 @@ mod tests { }; use crate::types::{InputMessage, MessageRequest, MessageResponse, OutputContentBlock, Usage}; - fn test_env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - } - #[test] fn path_builder_sanitizes_session_identifier() { let paths = PromptCachePaths::for_session("session:/with spaces"); diff --git a/rust/crates/api/src/providers/anthropic.rs b/rust/crates/api/src/providers/anthropic.rs index ccfbdfc800..0b9b2f5c75 100644 --- a/rust/crates/api/src/providers/anthropic.rs +++ b/rust/crates/api/src/providers/anthropic.rs @@ -1083,9 +1083,9 @@ struct AnthropicErrorBody { #[cfg(test)] mod tests { use super::{ALT_REQUEST_ID_HEADER, REQUEST_ID_HEADER}; + use crate::test_env_lock as env_lock; use std::io::{Read, Write}; use std::net::TcpListener; - use std::sync::{Mutex, OnceLock}; use std::thread; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -1097,13 +1097,6 @@ mod tests { }; use crate::types::{ContentBlockDelta, MessageRequest}; - fn env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - } - fn temp_config_home() -> std::path::PathBuf { std::env::temp_dir().join(format!( "api-oauth-test-{}-{}", diff --git a/rust/crates/api/src/providers/mod.rs b/rust/crates/api/src/providers/mod.rs index 5cf57b617b..9052904273 100644 --- a/rust/crates/api/src/providers/mod.rs +++ b/rust/crates/api/src/providers/mod.rs @@ -891,8 +891,8 @@ pub(crate) fn dotenv_value(key: &str) -> Option { #[cfg(test)] mod tests { + use crate::test_env_lock as env_lock; use std::ffi::OsString; - use std::sync::{Mutex, OnceLock}; use serde_json::json; @@ -910,17 +910,6 @@ mod tests { ProviderKind, ProviderWireProtocol, }; - /// Serializes every test in this module that mutates process-wide - /// environment variables so concurrent test threads cannot observe - /// each other's partially-applied state while probing the foreign - /// provider credential sniffer. - fn env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - } - /// Snapshot-restore guard for a single environment variable. Captures /// the original value on construction, applies the requested override /// (set or remove), and restores the original on drop so tests leave diff --git a/rust/crates/api/src/providers/openai_compat.rs b/rust/crates/api/src/providers/openai_compat.rs index 2c885e21ab..06e0071060 100644 --- a/rust/crates/api/src/providers/openai_compat.rs +++ b/rust/crates/api/src/providers/openai_compat.rs @@ -1863,6 +1863,7 @@ mod tests { StreamState, }; use crate::error::ApiError; + use crate::test_env_lock as env_lock; use crate::types::{ ContentBlockDelta, ContentBlockDeltaEvent, ContentBlockStartEvent, ContentBlockStopEvent, InputContentBlock, InputMessage, MessageRequest, OutputContentBlock, StreamEvent, @@ -1871,7 +1872,6 @@ mod tests { use serde_json::json; use std::borrow::Cow; use std::collections::BTreeMap; - use std::sync::{Mutex, OnceLock}; #[test] fn request_translation_uses_openai_compatible_shape() { @@ -2371,13 +2371,6 @@ mod tests { } } - fn env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) - .lock() - .expect("env lock") - } - #[test] fn normalizes_stop_reasons() { assert_eq!(normalize_finish_reason("stop"), "end_turn"); diff --git a/rust/crates/api/tests/proxy_integration.rs b/rust/crates/api/tests/proxy_integration.rs index 7e3906983f..2208f82f77 100644 --- a/rust/crates/api/tests/proxy_integration.rs +++ b/rust/crates/api/tests/proxy_integration.rs @@ -39,12 +39,13 @@ impl Drop for EnvVarGuard { fn proxy_config_from_env_reads_uppercase_proxy_vars() { // given let _lock = env_lock(); - let _http = EnvVarGuard::set("HTTP_PROXY", Some("http://proxy.corp:3128")); - let _https = EnvVarGuard::set("HTTPS_PROXY", Some("http://secure.corp:3129")); - let _no = EnvVarGuard::set("NO_PROXY", Some("localhost,127.0.0.1")); + // Clear aliases before setting values: environment keys are case-insensitive on Windows. let _http_lower = EnvVarGuard::set("http_proxy", None); let _https_lower = EnvVarGuard::set("https_proxy", None); let _no_lower = EnvVarGuard::set("no_proxy", None); + let _http = EnvVarGuard::set("HTTP_PROXY", Some("http://proxy.corp:3128")); + let _https = EnvVarGuard::set("HTTPS_PROXY", Some("http://secure.corp:3129")); + let _no = EnvVarGuard::set("NO_PROXY", Some("localhost,127.0.0.1")); // when let config = ProxyConfig::from_env(); @@ -127,13 +128,19 @@ fn proxy_config_from_env_treats_empty_values_as_unset() { fn build_client_with_env_proxy_config_succeeds() { // given let _lock = env_lock(); - let _http = EnvVarGuard::set("HTTP_PROXY", Some("http://proxy.corp:3128")); - let _https = EnvVarGuard::set("HTTPS_PROXY", Some("http://secure.corp:3129")); - let _no = EnvVarGuard::set("NO_PROXY", Some("localhost")); + // Clear aliases before setting values: environment keys are case-insensitive on Windows. let _http_lower = EnvVarGuard::set("http_proxy", None); let _https_lower = EnvVarGuard::set("https_proxy", None); let _no_lower = EnvVarGuard::set("no_proxy", None); + let _http = EnvVarGuard::set("HTTP_PROXY", Some("http://proxy.corp:3128")); + let _https = EnvVarGuard::set("HTTPS_PROXY", Some("http://secure.corp:3129")); + let _no = EnvVarGuard::set("NO_PROXY", Some("localhost")); let config = ProxyConfig::from_env(); + assert_eq!(config.http_proxy.as_deref(), Some("http://proxy.corp:3128")); + assert_eq!( + config.https_proxy.as_deref(), + Some("http://secure.corp:3129") + ); // when let result = build_http_client_with(&config); @@ -155,7 +162,7 @@ fn build_client_with_proxy_url_config_succeeds() { } #[test] -fn proxy_config_from_env_prefers_uppercase_over_lowercase() { +fn proxy_config_from_env_respects_platform_case_semantics() { // given let _lock = env_lock(); let _http_upper = EnvVarGuard::set("HTTP_PROXY", Some("http://upper.corp:3128")); @@ -169,5 +176,12 @@ fn proxy_config_from_env_prefers_uppercase_over_lowercase() { let config = ProxyConfig::from_env(); // then - assert_eq!(config.http_proxy.as_deref(), Some("http://upper.corp:3128")); + // Unix retains two keys and prefers uppercase. On Windows the second + // assignment replaces the same environment entry, regardless of casing. + let expected = if cfg!(windows) { + "http://lower.corp:3128" + } else { + "http://upper.corp:3128" + }; + assert_eq!(config.http_proxy.as_deref(), Some(expected)); } diff --git a/rust/crates/plugins/src/hooks.rs b/rust/crates/plugins/src/hooks.rs index 9ceb94e390..cc9b498bea 100644 --- a/rust/crates/plugins/src/hooks.rs +++ b/rust/crates/plugins/src/hooks.rs @@ -1,5 +1,4 @@ use std::ffi::OsStr; -#[cfg(not(windows))] use std::path::Path; use std::process::Command; @@ -282,8 +281,18 @@ fn format_hook_warning(command: &str, code: i32, stdout: Option<&str>, stderr: & fn shell_command(command: &str) -> CommandWithStdin { #[cfg(windows)] let command_builder = { + use std::os::windows::process::CommandExt; + + // This is a configured shell command, not a single argv value. cmd + // does not understand the C-runtime backslash escaping used by arg(). + let command = if Path::new(command).is_file() { + format!("\"{command}\"") + } else { + command.to_string() + }; let mut command_builder = Command::new("cmd"); - command_builder.arg("/C").arg(command); + command_builder.args(["/D", "/S", "/C"]); + command_builder.raw_arg(format!("\"{command}\"")); CommandWithStdin::new(command_builder) }; @@ -381,6 +390,14 @@ mod tests { std::env::temp_dir().join(format!("plugins-hook-runner-{label}-{nanos}")) } + fn plugin_hook(message: &str, code: i32) -> String { + if cfg!(windows) { + format!("echo {message}& exit /b {code}") + } else { + format!("printf '%s' '{message}'; exit {code}") + } + } + fn make_executable(path: &Path) { #[cfg(unix)] { @@ -403,34 +420,34 @@ mod tests { fs::create_dir_all(root.join(".claude-plugin")).expect("manifest dir"); fs::create_dir_all(root.join("hooks")).expect("hooks dir"); - let pre_path = root.join("hooks").join("pre.sh"); - fs::write( - &pre_path, - format!("#!/bin/sh\nprintf '%s\\n' '{pre_message}'\n"), - ) - .expect("write pre hook"); - make_executable(&pre_path); - - let post_path = root.join("hooks").join("post.sh"); - fs::write( - &post_path, - format!("#!/bin/sh\nprintf '%s\\n' '{post_message}'\n"), - ) - .expect("write post hook"); - make_executable(&post_path); - - let failure_path = root.join("hooks").join("failure.sh"); - fs::write( - &failure_path, - format!("#!/bin/sh\nprintf '%s\\n' '{failure_message}'\n"), - ) - .expect("write failure hook"); - make_executable(&failure_path); + let extension = if cfg!(windows) { "cmd" } else { "sh" }; + for (phase, message) in [ + ("pre", pre_message), + ("post", post_message), + ("failure", failure_message), + ] { + let path = root.join("hooks").join(format!("{phase}.{extension}")); + let script = if cfg!(windows) { + format!("@echo off\r\necho {message}\r\n") + } else { + format!("#!/bin/sh\nprintf '%s\\n' '{message}'\n") + }; + fs::write(&path, script).expect("write hook"); + make_executable(&path); + } fs::write( root.join(".claude-plugin").join("plugin.json"), - format!( - "{{\n \"name\": \"{name}\",\n \"version\": \"1.0.0\",\n \"description\": \"hook plugin\",\n \"hooks\": {{\n \"PreToolUse\": [\"./hooks/pre.sh\"],\n \"PostToolUse\": [\"./hooks/post.sh\"],\n \"PostToolUseFailure\": [\"./hooks/failure.sh\"]\n }}\n}}" - ), + serde_json::json!({ + "name": name, + "version": "1.0.0", + "description": "hook plugin", + "hooks": { + "PreToolUse": [format!("./hooks/pre.{extension}")], + "PostToolUse": [format!("./hooks/post.{extension}")], + "PostToolUseFailure": [format!("./hooks/failure.{extension}")] + } + }) + .to_string(), ) .expect("write plugin manifest"); } @@ -439,7 +456,7 @@ mod tests { fn collects_and_runs_hooks_from_enabled_plugins() { // given let config_home = temp_dir("config"); - let first_source_root = temp_dir("source-a"); + let first_source_root = temp_dir("source a"); let second_source_root = temp_dir("source-b"); write_hook_plugin( &first_source_root, @@ -500,7 +517,7 @@ mod tests { fn pre_tool_use_denies_when_plugin_hook_exits_two() { // given let runner = HookRunner::new(crate::PluginHooks { - pre_tool_use: vec!["printf 'blocked by plugin'; exit 2".to_string()], + pre_tool_use: vec![plugin_hook("blocked by plugin", 2)], post_tool_use: Vec::new(), post_tool_use_failure: Vec::new(), }); @@ -513,13 +530,28 @@ mod tests { assert_eq!(result.messages(), &["blocked by plugin".to_string()]); } + #[test] + fn preserves_quoted_arguments_in_plugin_hooks() { + let python = if cfg!(windows) { "python" } else { "python3" }; + let runner = HookRunner::new(crate::PluginHooks { + pre_tool_use: vec![format!( + "{python} -c \"import sys; sys.stdout.write('quoted plugin hook'); sys.exit(2)\"" + )], + post_tool_use: Vec::new(), + post_tool_use_failure: Vec::new(), + }); + let result = runner.run_pre_tool_use("Read", r#"{"path":"README.md"}"#); + assert!(result.is_denied(), "{result:?}"); + assert_eq!(result.messages(), &["quoted plugin hook".to_string()]); + } + #[test] fn propagates_plugin_hook_failures() { // given let runner = HookRunner::new(crate::PluginHooks { pre_tool_use: vec![ - "printf 'broken plugin hook'; exit 1".to_string(), - "printf 'later plugin hook'".to_string(), + plugin_hook("broken plugin hook", 1), + plugin_hook("later plugin hook", 0), ], post_tool_use: Vec::new(), post_tool_use_failure: Vec::new(), diff --git a/rust/crates/plugins/src/lib.rs b/rust/crates/plugins/src/lib.rs index 95a87f36ed..e566c6959a 100644 --- a/rust/crates/plugins/src/lib.rs +++ b/rust/crates/plugins/src/lib.rs @@ -2525,25 +2525,28 @@ mod tests { } fn write_tool_plugin_with_name(root: &Path, name: &str, version: &str, tool_name: &str) { - let script_path = root.join("tools").join("echo-json.sh"); + let script_path = root.join("tools").join("echo-json.py"); write_file( &script_path, - "#!/bin/sh\nINPUT=$(cat)\nprintf '{\"plugin\":\"%s\",\"tool\":\"%s\",\"input\":%s}\\n' \"$CLAWD_PLUGIN_ID\" \"$CLAWD_TOOL_NAME\" \"$INPUT\"\n", + r#"import json, os, sys +print(json.dumps({"plugin": os.environ["CLAWD_PLUGIN_ID"], "tool": os.environ["CLAWD_TOOL_NAME"], "input": json.load(sys.stdin)})) +"#, ); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - - let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("chmod"); - } write_file( root.join(MANIFEST_RELATIVE_PATH).as_path(), - format!( - "{{\n \"name\": \"{name}\",\n \"version\": \"{version}\",\n \"description\": \"tool plugin\",\n \"tools\": [\n {{\n \"name\": \"{tool_name}\",\n \"description\": \"Echo JSON input\",\n \"inputSchema\": {{\"type\": \"object\", \"properties\": {{\"message\": {{\"type\": \"string\"}}}}, \"required\": [\"message\"], \"additionalProperties\": false}},\n \"command\": \"./tools/echo-json.sh\",\n \"requiredPermission\": \"workspace-write\"\n }}\n ]\n}}" - ) - .as_str(), + &serde_json::json!({ + "name": name, + "version": version, + "description": "tool plugin", + "tools": [{ + "name": tool_name, + "description": "Echo JSON input", + "inputSchema": {"type": "object", "properties": {"message": {"type": "string"}}, "required": ["message"], "additionalProperties": false}, + "command": if cfg!(windows) { "python" } else { "python3" }, + "args": ["tools/echo-json.py"], + "requiredPermission": "workspace-write" + }] + }).to_string(), ); } diff --git a/rust/crates/runtime/Cargo.toml b/rust/crates/runtime/Cargo.toml index 38436133e7..1e6c16d3f2 100644 --- a/rust/crates/runtime/Cargo.toml +++ b/rust/crates/runtime/Cargo.toml @@ -7,6 +7,7 @@ publish.workspace = true [dependencies] sha2 = "0.10" +getrandom = "0.2" glob = "0.3" plugins = { path = "../plugins" } regex = "1" diff --git a/rust/crates/runtime/src/bash.rs b/rust/crates/runtime/src/bash.rs index c2a35367db..d92b5228a9 100644 --- a/rust/crates/runtime/src/bash.rs +++ b/rust/crates/runtime/src/bash.rs @@ -293,6 +293,41 @@ fn sandbox_status_for_input(input: &BashCommandInput, cwd: &std::path::Path) -> resolve_sandbox_status_for_request(&request, cwd) } +// Git for Windows normally adds Git/cmd, but not Git/bin, to PATH. +// Prefer a native shell there rather than the Windows WSL bash launcher. +fn posix_shell() -> std::path::PathBuf { + #[cfg(windows)] + { + if let Some(path) = env::var_os("PATH") { + for directory in env::split_paths(&path) { + let shell = directory.join("sh.exe"); + if shell.is_file() { + return shell; + } + if directory.join("git.exe").is_file() { + if let Some(root) = directory.parent() { + let shell = root.join("bin").join("sh.exe"); + if shell.is_file() { + return shell; + } + } + } + } + } + for variable in ["ProgramFiles", "ProgramFiles(x86)", "LOCALAPPDATA"] { + if let Some(root) = env::var_os(variable) { + for relative in ["Git/bin/sh.exe", "Programs/Git/bin/sh.exe"] { + let shell = std::path::PathBuf::from(&root).join(relative); + if shell.is_file() { + return shell; + } + } + } + } + } + std::path::PathBuf::from("sh") +} + fn prepare_command( command: &str, cwd: &std::path::Path, @@ -311,7 +346,7 @@ fn prepare_command( return prepared; } - let mut prepared = Command::new("sh"); + let mut prepared = Command::new(posix_shell()); prepared.arg("-lc").arg(command).current_dir(cwd); if sandbox_status.filesystem_active { prepared.env("HOME", cwd.join(".sandbox-home")); @@ -337,7 +372,7 @@ fn prepare_tokio_command( cmd.envs(launcher.env); cmd } else { - let mut cmd = TokioCommand::new("sh"); + let mut cmd = TokioCommand::new(posix_shell()); cmd.arg("-lc").arg(command); if sandbox_status.filesystem_active { cmd.env("HOME", cwd.join(".sandbox-home")); diff --git a/rust/crates/runtime/src/config.rs b/rust/crates/runtime/src/config.rs index fe95bf2bca..67c274cb35 100644 --- a/rust/crates/runtime/src/config.rs +++ b/rust/crates/runtime/src/config.rs @@ -2817,12 +2817,12 @@ mod tests { let command = format!("echo compromised >> {}", marker.display()); fs::write( &project_legacy_settings, - format!(r#"{{"hooks":{{"PreToolUse":["{command}"]}}}}"#), + serde_json::json!({"hooks": {"PreToolUse": [command]}}).to_string(), ) .expect("write untrusted legacy project hook"); fs::write( &project_settings, - format!(r#"{{"hooks":{{"PreToolUse":["{command}"]}}}}"#), + serde_json::json!({"hooks": {"PreToolUse": [command]}}).to_string(), ) .expect("write untrusted project hook"); diff --git a/rust/crates/runtime/src/conversation.rs b/rust/crates/runtime/src/conversation.rs index 0972a69851..7cc14c7ad0 100644 --- a/rust/crates/runtime/src/conversation.rs +++ b/rust/crates/runtime/src/conversation.rs @@ -1705,9 +1705,9 @@ mod tests { )); let hook_command = format!("echo compromised >> \"{}\"", marker.display()); let features = RuntimeFeatureConfig::default().with_hooks(RuntimeHookConfig::new( - vec![shell_snippet(&hook_command)], - vec![shell_snippet(&hook_command)], - vec![shell_snippet(&hook_command)], + vec![hook_command.clone()], + vec![hook_command.clone()], + vec![hook_command.clone()], )); let mut runtime = ConversationRuntime::new_with_features( Session::new(), @@ -1858,7 +1858,7 @@ mod tests { PermissionPolicy::new(PermissionMode::DangerFullAccess), vec!["system".to_string()], &RuntimeFeatureConfig::default().with_hooks(RuntimeHookConfig::new( - vec![shell_snippet("printf 'blocked by hook'; exit 2")], + vec![crate::test_hook("blocked by hook", "", 2, 0)], Vec::new(), Vec::new(), )), @@ -1921,7 +1921,7 @@ mod tests { PermissionPolicy::new(PermissionMode::DangerFullAccess), vec!["system".to_string()], &RuntimeFeatureConfig::default().with_hooks(RuntimeHookConfig::new( - vec![shell_snippet("printf 'broken hook'; exit 1")], + vec![crate::test_hook("broken hook", "", 1, 0)], Vec::new(), Vec::new(), )), @@ -1990,8 +1990,8 @@ mod tests { PermissionPolicy::new(PermissionMode::DangerFullAccess), vec!["system".to_string()], &RuntimeFeatureConfig::default().with_hooks(RuntimeHookConfig::new( - vec![shell_snippet("printf 'pre hook ran'")], - vec![shell_snippet("printf 'post hook ran'")], + vec![crate::test_hook("pre hook ran", "", 0, 0)], + vec![crate::test_hook("post hook ran", "", 0, 0)], Vec::new(), )), ); @@ -2068,8 +2068,8 @@ mod tests { vec!["system".to_string()], &RuntimeFeatureConfig::default().with_hooks(RuntimeHookConfig::new( Vec::new(), - vec![shell_snippet("printf 'post hook should not run'")], - vec![shell_snippet("printf 'failure hook ran'")], + vec![crate::test_hook("post hook should not run", "", 0, 0)], + vec![crate::test_hook("failure hook ran", "", 0, 0)], )), ); @@ -2263,16 +2263,6 @@ mod tests { std::env::temp_dir().join(format!("runtime-conversation-{label}-{nanos}.json")) } - #[cfg(windows)] - fn shell_snippet(script: &str) -> String { - script.replace('\'', "\"") - } - - #[cfg(not(windows))] - fn shell_snippet(script: &str) -> String { - script.to_string() - } - #[test] fn auto_compacts_when_cumulative_input_threshold_is_crossed() { struct SimpleApi; diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index 220de06cf0..f64e3665b3 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -649,6 +649,11 @@ fn glob_search_impl( base_dir.join(pattern).to_string_lossy().into_owned() }; + #[cfg(windows)] + let search_pattern = normalize_for_comparison(Path::new(&search_pattern)) + .to_string_lossy() + .replace('\\', "/"); + // The `glob` crate does not support brace expansion ({a,b,c}). // Expand braces into multiple patterns so patterns like // `Assets/**/*.{cs,uxml,uss}` work correctly. @@ -1123,14 +1128,15 @@ fn expand_braces(pattern: &str) -> Vec { #[cfg(test)] mod tests { - use std::path::PathBuf; + use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; + #[cfg(unix)] + use super::write_file_in_workspace; use super::{ component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search, grep_search, is_symlink_escape, normalize_for_comparison, read_file, - read_file_in_workspace, write_file, write_file_in_workspace, GrepSearchInput, - MAX_WRITE_SIZE, + read_file_in_workspace, write_file, GrepSearchInput, MAX_WRITE_SIZE, }; fn temp_path(name: &str) -> std::path::PathBuf { @@ -1217,9 +1223,9 @@ mod tests { let outside = temp_path("symlink-target.txt"); std::fs::write(&outside, "target content").expect("target should write"); - let link_path = workspace.join("escape-link.txt"); #[cfg(unix)] { + let link_path = workspace.join("escape-link.txt"); std::os::unix::fs::symlink(&outside, &link_path).expect("symlink should create"); assert!(is_symlink_escape(&link_path, &workspace).expect("check should succeed")); } @@ -1476,11 +1482,11 @@ mod tests { assert!(result .filenames .iter() - .any(|path| path.ends_with("src/AGENTS.md"))); + .any(|path| Path::new(path).ends_with("src/AGENTS.md"))); assert!(result .filenames .iter() - .any(|path| path.ends_with("docs/AGENTS.md"))); + .any(|path| Path::new(path).ends_with("docs/AGENTS.md"))); assert!(!result .filenames .iter() diff --git a/rust/crates/runtime/src/hooks.rs b/rust/crates/runtime/src/hooks.rs index 2d9f25e72b..d315b9c60d 100644 --- a/rust/crates/runtime/src/hooks.rs +++ b/rust/crates/runtime/src/hooks.rs @@ -742,8 +742,13 @@ fn format_hook_failure(command: &str, code: i32, stdout: Option<&str>, stderr: & fn shell_command(command: &str) -> CommandWithStdin { #[cfg(windows)] let command_builder = { + use std::os::windows::process::CommandExt; + + // This is a configured shell command, not a single argv value. cmd + // does not understand the C-runtime backslash escaping used by arg(). let mut command_builder = Command::new("cmd"); - command_builder.arg("/C").arg(command); + command_builder.args(["/D", "/S", "/C"]); + command_builder.raw_arg(format!("\"{command}\"")); CommandWithStdin::new(command_builder) }; @@ -845,7 +850,7 @@ mod tests { #[test] fn allows_exit_code_zero_and_captures_stdout() { let runner = HookRunner::new(RuntimeHookConfig::new( - vec![shell_snippet("printf 'pre ok'")], + vec![crate::test_hook("pre ok", "", 0, 0)], Vec::new(), Vec::new(), )); @@ -859,13 +864,13 @@ mod tests { fn object_style_hook_matchers_filter_runtime_execution() { let runner = HookRunner::new(RuntimeHookConfig::from_hook_commands( vec![ - RuntimeHookCommand::new(shell_snippet("printf 'legacy'")), + RuntimeHookCommand::new(crate::test_hook("legacy", "", 0, 0)), RuntimeHookCommand::with_matcher( - shell_snippet("printf 'bash only'"), + crate::test_hook("bash only", "", 0, 0), Some("Bash".to_string()), ), RuntimeHookCommand::with_matcher( - shell_snippet("printf 'read only'"), + crate::test_hook("read only", "", 0, 0), Some("Read*".to_string()), ), ], @@ -889,7 +894,7 @@ mod tests { #[test] fn denies_exit_code_two() { let runner = HookRunner::new(RuntimeHookConfig::new( - vec![shell_snippet("printf 'blocked by hook'; exit 2")], + vec![crate::test_hook("blocked by hook", "", 2, 0)], Vec::new(), Vec::new(), )); @@ -904,7 +909,7 @@ mod tests { fn propagates_other_non_zero_statuses_as_failures() { let runner = HookRunner::from_feature_config(&RuntimeFeatureConfig::default().with_hooks( RuntimeHookConfig::new( - vec![shell_snippet("printf 'warning hook'; exit 1")], + vec![crate::test_hook("warning hook", "", 1, 0)], Vec::new(), Vec::new(), ), @@ -925,8 +930,11 @@ mod tests { #[test] fn parses_pre_hook_permission_override_and_updated_input() { let runner = HookRunner::new(RuntimeHookConfig::new( - vec![shell_snippet( - r#"printf '%s' '{"systemMessage":"updated","hookSpecificOutput":{"permissionDecision":"allow","permissionDecisionReason":"hook ok","updatedInput":{"command":"git status"}}}'"#, + vec![crate::test_hook( + r#"{"systemMessage":"updated","hookSpecificOutput":{"permissionDecision":"allow","permissionDecisionReason":"hook ok","updatedInput":{"command":"git status"}}}"#, + "", + 0, + 0, )], Vec::new(), Vec::new(), @@ -949,7 +957,7 @@ mod tests { let runner = HookRunner::new(RuntimeHookConfig::new( Vec::new(), Vec::new(), - vec![shell_snippet("printf 'failure hook ran'")], + vec![crate::test_hook("failure hook ran", "", 0, 0)], )); // when @@ -968,8 +976,8 @@ mod tests { Vec::new(), Vec::new(), vec![ - shell_snippet("printf 'broken failure hook'; exit 1"), - shell_snippet("printf 'later failure hook'"), + crate::test_hook("broken failure hook", "", 1, 0), + crate::test_hook("later failure hook", "", 0, 0), ], )); @@ -994,8 +1002,8 @@ mod tests { // given let runner = HookRunner::new(RuntimeHookConfig::new( vec![ - shell_snippet("printf 'first'"), - shell_snippet("printf 'second'"), + crate::test_hook("first", "", 0, 0), + crate::test_hook("second", "", 0, 0), ], Vec::new(), Vec::new(), @@ -1022,7 +1030,7 @@ mod tests { event: HookEvent::PreToolUse, command, .. - } if command == "printf 'first'" + } if command == &crate::test_hook("first", "", 0, 0) )); assert!(matches!( &reporter.events[1], @@ -1030,7 +1038,7 @@ mod tests { event: HookEvent::PreToolUse, command, .. - } if command == "printf 'first'" + } if command == &crate::test_hook("first", "", 0, 0) )); assert!(matches!( &reporter.events[2], @@ -1038,7 +1046,7 @@ mod tests { event: HookEvent::PreToolUse, command, .. - } if command == "printf 'second'" + } if command == &crate::test_hook("second", "", 0, 0) )); assert!(matches!( &reporter.events[3], @@ -1046,7 +1054,7 @@ mod tests { event: HookEvent::PreToolUse, command, .. - } if command == "printf 'second'" + } if command == &crate::test_hook("second", "", 0, 0) )); } @@ -1055,8 +1063,8 @@ mod tests { // given let runner = HookRunner::new(RuntimeHookConfig::new( vec![ - shell_snippet("printf 'broken'; exit 1"), - shell_snippet("printf 'later'"), + crate::test_hook("broken", "", 1, 0), + crate::test_hook("later", "", 0, 0), ], Vec::new(), Vec::new(), @@ -1077,8 +1085,11 @@ mod tests { #[test] fn malformed_nonempty_hook_output_reports_explicit_diagnostic_with_previews() { let runner = HookRunner::new(RuntimeHookConfig::new( - vec![shell_snippet( - "printf '{not-json\nsecond line'; printf 'stderr warning' >&2; exit 1", + vec![crate::test_hook( + "{not-json\nsecond line", + "stderr warning", + 1, + 0, )], Vec::new(), Vec::new(), @@ -1091,8 +1102,16 @@ mod tests { assert!(rendered.contains("hook_invalid_json:")); assert!(rendered.contains("phase=PreToolUse")); assert!(rendered.contains("tool=Edit")); - assert!(rendered.contains("command=printf '{not-json")); - assert!(rendered.contains("printf 'stderr warning' >&2; exit 1")); + assert!(rendered.contains(&format!( + "command={}", + super::bounded_hook_preview(&crate::test_hook( + "{not-json\nsecond line", + "stderr warning", + 1, + 0 + )) + .expect("command preview") + ))); assert!(rendered.contains("detail=key must be a string")); assert!(rendered.contains("stdout_preview={not-json")); assert!(rendered.contains("second line stderr_preview=stderr warning")); @@ -1102,7 +1121,7 @@ mod tests { #[test] fn abort_signal_cancels_long_running_hook_and_reports_progress() { let runner = HookRunner::new(RuntimeHookConfig::new( - vec![shell_snippet("sleep 5")], + vec![crate::test_hook("", "", 0, 5)], Vec::new(), Vec::new(), )); @@ -1138,14 +1157,4 @@ mod tests { } ))); } - - #[cfg(windows)] - fn shell_snippet(script: &str) -> String { - script.replace('\'', "\"") - } - - #[cfg(not(windows))] - fn shell_snippet(script: &str) -> String { - script.to_string() - } } diff --git a/rust/crates/runtime/src/lib.rs b/rust/crates/runtime/src/lib.rs index 50959ac300..5af860ce0f 100644 --- a/rust/crates/runtime/src/lib.rs +++ b/rust/crates/runtime/src/lib.rs @@ -214,3 +214,17 @@ pub(crate) fn test_env_lock() -> std::sync::MutexGuard<'static, ()> { .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) } + +#[cfg(test)] +pub(crate) fn test_hook(stdout: &str, stderr: &str, exit_code: i32, delay: u32) -> String { + let python = if cfg!(windows) { "python" } else { "python3" }; + let encode = |text: &str| { + text.bytes() + .map(|byte| format!("{byte:02x}")) + .collect::() + }; + format!( + "{python} -c \"import sys,time; time.sleep({delay}); sys.stdout.write(bytes.fromhex('{}').decode()); sys.stderr.write(bytes.fromhex('{}').decode()); sys.exit({exit_code})\"", + encode(stdout), encode(stderr) + ) +} diff --git a/rust/crates/runtime/src/mcp_stdio.rs b/rust/crates/runtime/src/mcp_stdio.rs index 35efa7eecd..0a39674d13 100644 --- a/rust/crates/runtime/src/mcp_stdio.rs +++ b/rust/crates/runtime/src/mcp_stdio.rs @@ -1443,7 +1443,6 @@ mod tests { use std::collections::BTreeMap; use std::fs; use std::io::ErrorKind; - use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -1466,6 +1465,9 @@ mod tests { }; use crate::McpLifecyclePhase; + // Fixtures are interpreter inputs, not directly executable Unix scripts. + const TEST_PYTHON: &str = if cfg!(windows) { "python" } else { "python3" }; + fn temp_dir() -> PathBuf { static NEXT_TEMP_DIR_ID: AtomicU64 = AtomicU64::new(0); let nanos = SystemTime::now() @@ -1479,15 +1481,19 @@ mod tests { fn write_echo_script() -> PathBuf { let root = temp_dir(); fs::create_dir_all(&root).expect("temp dir"); - let script_path = root.join("echo-mcp.sh"); - fs::write( - &script_path, - "#!/bin/sh\nprintf 'READY:%s\\n' \"$MCP_TEST_TOKEN\"\nIFS= read -r line\nprintf 'ECHO:%s\\n' \"$line\"\n", - ) - .expect("write script"); - let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("chmod"); + let script_path = root.join("echo-mcp.py"); + let script = [ + "import os, sys", + r"ready = ('READY:' + os.environ['MCP_TEST_TOKEN'] + '\n').encode()", + "sys.stdout.buffer.write(ready)", + "sys.stdout.buffer.flush()", + "line = sys.stdin.buffer.readline()", + r"sys.stdout.buffer.write(b'ECHO:' + line.rstrip(b'\r\n') + b'\n')", + "sys.stdout.buffer.flush()", + "", + ] + .join("\n"); + fs::write(&script_path, script).expect("write script"); script_path } @@ -1531,9 +1537,6 @@ mod tests { ] .join("\n"); fs::write(&script_path, script).expect("write script"); - let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("chmod"); script_path } @@ -1665,9 +1668,6 @@ mod tests { ] .join("\n"); fs::write(&script_path, script).expect("write script"); - let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("chmod"); script_path } @@ -1790,9 +1790,6 @@ mod tests { ] .join("\n"); fs::write(&script_path, script).expect("write script"); - let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("chmod"); script_path } @@ -1801,7 +1798,7 @@ mod tests { required: false, scope: ConfigSource::Local, config: McpServerConfig::Stdio(McpStdioServerConfig { - command: "/bin/sh".to_string(), + command: TEST_PYTHON.to_string(), args: vec![script_path.to_string_lossy().into_owned()], env: BTreeMap::from([("MCP_TEST_TOKEN".to_string(), "secret-value".to_string())]), tool_call_timeout_ms: None, @@ -1819,7 +1816,7 @@ mod tests { env: BTreeMap, ) -> crate::mcp_client::McpStdioTransport { crate::mcp_client::McpStdioTransport { - command: "python3".to_string(), + command: TEST_PYTHON.to_string(), args: vec![script_path.to_string_lossy().into_owned()], env, tool_call_timeout_ms: None, @@ -1869,7 +1866,7 @@ mod tests { required: false, scope: ConfigSource::Local, config: McpServerConfig::Stdio(McpStdioServerConfig { - command: "python3".to_string(), + command: TEST_PYTHON.to_string(), args: vec![script_path.to_string_lossy().into_owned()], env, tool_call_timeout_ms: None, @@ -2089,7 +2086,7 @@ mod tests { runtime.block_on(async { let script_path = write_echo_script(); let transport = crate::mcp_client::McpStdioTransport { - command: "/bin/sh".to_string(), + command: TEST_PYTHON.to_string(), args: vec![script_path.to_string_lossy().into_owned()], env: BTreeMap::from([("MCP_TEST_TOKEN".to_string(), "direct-secret".to_string())]), tool_call_timeout_ms: None, @@ -2349,7 +2346,7 @@ mod tests { required: false, scope: ConfigSource::Local, config: McpServerConfig::Stdio(McpStdioServerConfig { - command: "python3".to_string(), + command: TEST_PYTHON.to_string(), args: vec![script_path.to_string_lossy().into_owned()], env: BTreeMap::from([( "MCP_TOOL_CALL_DELAY_MS".to_string(), @@ -2403,7 +2400,7 @@ mod tests { required: false, scope: ConfigSource::Local, config: McpServerConfig::Stdio(McpStdioServerConfig { - command: "python3".to_string(), + command: TEST_PYTHON.to_string(), args: vec![script_path.to_string_lossy().into_owned()], env: BTreeMap::from([( "MCP_INVALID_TOOL_CALL_RESPONSE".to_string(), @@ -2714,9 +2711,6 @@ mod tests { ] .join("\n"); fs::write(&script_path, script).expect("write script"); - let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("chmod"); script_path } @@ -2742,8 +2736,8 @@ mod tests { required: true, scope: ConfigSource::Local, config: McpServerConfig::Stdio(McpStdioServerConfig { - command: broken_script_path.display().to_string(), - args: Vec::new(), + command: TEST_PYTHON.to_string(), + args: vec![broken_script_path.to_string_lossy().into_owned()], env: BTreeMap::new(), tool_call_timeout_ms: None, }), diff --git a/rust/crates/runtime/src/mcp_tool_bridge.rs b/rust/crates/runtime/src/mcp_tool_bridge.rs index 5b7c326ca9..de6ad30f4b 100644 --- a/rust/crates/runtime/src/mcp_tool_bridge.rs +++ b/rust/crates/runtime/src/mcp_tool_bridge.rs @@ -314,7 +314,6 @@ impl McpToolRegistry { mod tests { use std::collections::BTreeMap; use std::fs; - use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -324,6 +323,9 @@ mod tests { ConfigSource, McpServerConfig, McpStdioServerConfig, ScopedMcpServerConfig, }; + // Fixtures are interpreter inputs, not directly executable Unix scripts. + const TEST_PYTHON: &str = if cfg!(windows) { "python" } else { "python3" }; + fn temp_dir() -> PathBuf { static NEXT_TEMP_DIR_ID: AtomicU64 = AtomicU64::new(0); let nanos = SystemTime::now() @@ -430,9 +432,6 @@ mod tests { ] .join("\n"); fs::write(&script_path, script).expect("write script"); - let mut permissions = fs::metadata(&script_path).expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("chmod"); script_path } @@ -445,7 +444,7 @@ mod tests { required: false, scope: ConfigSource::Local, config: McpServerConfig::Stdio(McpStdioServerConfig { - command: "python3".to_string(), + command: TEST_PYTHON.to_string(), args: vec![script_path.to_string_lossy().into_owned()], env: BTreeMap::from([ ("MCP_SERVER_LABEL".to_string(), server_name.to_string()), diff --git a/rust/crates/runtime/src/oauth.rs b/rust/crates/runtime/src/oauth.rs index aa3ca158c7..30e05bf4f3 100644 --- a/rust/crates/runtime/src/oauth.rs +++ b/rust/crates/runtime/src/oauth.rs @@ -1,6 +1,6 @@ use std::collections::BTreeMap; -use std::fs::{self, File}; -use std::io::{self, Read}; +use std::fs; +use std::io; use std::path::PathBuf; use serde::{Deserialize, Serialize}; @@ -326,7 +326,7 @@ pub fn parse_oauth_callback_query(query: &str) -> Result io::Result { let mut buffer = vec![0_u8; bytes]; - File::open("/dev/urandom")?.read_exact(&mut buffer)?; + getrandom::getrandom(&mut buffer).map_err(|error| io::Error::other(error.to_string()))?; Ok(base64url_encode(&buffer)) } diff --git a/rust/crates/runtime/src/session.rs b/rust/crates/runtime/src/session.rs index 2ecfd97dab..8590affd55 100644 --- a/rust/crates/runtime/src/session.rs +++ b/rust/crates/runtime/src/session.rs @@ -262,6 +262,12 @@ impl Session { pub fn load_from_path(path: impl AsRef) -> Result { let path = path.as_ref(); + if fs::metadata(path)?.is_dir() { + return Err(SessionError::Io(std::io::Error::new( + std::io::ErrorKind::IsADirectory, + format!("session_path_is_directory: {}", path.display()), + ))); + } let contents = fs::read_to_string(path)?; let session = match JsonValue::parse(&contents) { Ok(value) diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 273b7f330c..02526affba 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -460,7 +460,10 @@ fn classify_error_kind(message: &str) -> &'static str { // error message is "failed to restore session: legacy session is missing workspace // binding: ...", so the specific arm must be checked first. "legacy_session_no_workspace_binding" - } else if message.contains("Is a directory") || message.contains("os error 21") { + } else if message.contains("session_path_is_directory:") + || message.contains("Is a directory") + || message.contains("os error 21") + { // #787: --resume given a directory path instead of a .jsonl file "session_path_is_directory" } else if message.contains("failed to restore session") { @@ -517,7 +520,10 @@ fn classify_error_kind(message: &str) -> &'static str { "api_http_error" } else if message.contains("mcpServers") { "malformed_mcp_config" - } else if message.contains(".claw/settings.json") || message.contains(".claw.json") { + } else if message.contains(".claw/settings.json") + || message.contains(r".claw\settings.json") + || message.contains(".claw.json") + { // #763: config file JSON parse / validation errors (e.g. unterminated string, type mismatch) "config_parse_error" } else if message.starts_with("empty prompt") { @@ -16697,7 +16703,10 @@ mod tests { // Phase 1 contract: workspace/git/sandbox fields are still populated // (independent of config parse). Sandbox falls back to defaults. - assert_eq!(context.cwd, cwd.canonicalize().unwrap_or(cwd.clone())); + assert_eq!( + context.cwd.canonicalize().expect("canonical cwd"), + cwd.canonicalize().expect("canonical workspace") + ); assert_eq!( context.loaded_config_files, 0, "loaded_config_files should be 0 when config parse fails" @@ -17038,6 +17047,16 @@ mod tests { classify_error_kind("failed to restore session: Is a directory (os error 21)"), "session_path_is_directory" ); + assert_eq!( + classify_error_kind( + r"failed to restore session: session_path_is_directory: C:\sessions" + ), + "session_path_is_directory" + ); + assert_eq!( + classify_error_kind(r"C:\workspace\.claw\settings.json: model must be a string"), + "config_parse_error" + ); assert_eq!( classify_error_kind("unrecognized argument `--foo` for subcommand `doctor`"), "cli_parse" @@ -20315,21 +20334,19 @@ UU conflicted.rs", write_mcp_server_fixture(&script_path); fs::write( config_home.join("settings.json"), - format!( - r#"{{ - "mcpServers": {{ - "alpha": {{ - "command": "python3", - "args": ["{}"] - }}, - "broken": {{ - "command": "python3", - "args": ["-c", "import sys; sys.exit(0)"] - }} - }} - }}"#, - script_path.to_string_lossy() - ), + serde_json::json!({ + "mcpServers": { + "alpha": { + "command": if cfg!(windows) { "python" } else { "python3" }, + "args": [script_path] + }, + "broken": { + "command": if cfg!(windows) { "python" } else { "python3" }, + "args": ["-c", "import sys; sys.exit(0)"] + } + } + }) + .to_string(), ) .expect("write mcp settings"); diff --git a/rust/crates/rusty-claude-cli/tests/compact_output.rs b/rust/crates/rusty-claude-cli/tests/compact_output.rs index 8509474db4..29b9e905cf 100644 --- a/rust/crates/rusty-claude-cli/tests/compact_output.rs +++ b/rust/crates/rusty-claude-cli/tests/compact_output.rs @@ -1,5 +1,7 @@ #![allow(clippy::while_let_on_iterator)] +mod support; + use std::fs; use std::io::Write; use std::path::PathBuf; @@ -655,12 +657,12 @@ fn run_claw( command .current_dir(cwd) .env_clear() + .envs(support::process_environment()) .env("ANTHROPIC_API_KEY", "test-compact-key") .env("ANTHROPIC_BASE_URL", base_url) .env("CLAW_CONFIG_HOME", config_home) .env("HOME", home) .env("NO_COLOR", "1") - .env("PATH", "/usr/bin:/bin") .args(args); command.output().expect("claw should launch") } @@ -676,12 +678,12 @@ fn run_claw_with_stdin( let mut child = Command::new(env!("CARGO_BIN_EXE_claw")) .current_dir(cwd) .env_clear() + .envs(support::process_environment()) .env("ANTHROPIC_API_KEY", "test-compact-key") .env("ANTHROPIC_BASE_URL", base_url) .env("CLAW_CONFIG_HOME", config_home) .env("HOME", home) .env("NO_COLOR", "1") - .env("PATH", "/usr/bin:/bin") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -708,10 +710,10 @@ fn run_claw_closed_stdin_with_timeout( let mut child = Command::new(env!("CARGO_BIN_EXE_claw")) .current_dir(cwd) .env_clear() + .envs(support::process_environment()) .env("CLAW_CONFIG_HOME", config_home) .env("HOME", home) .env("NO_COLOR", "1") - .env("PATH", "/usr/bin:/bin") .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) diff --git a/rust/crates/rusty-claude-cli/tests/compact_repl_panic.rs b/rust/crates/rusty-claude-cli/tests/compact_repl_panic.rs index e930cf4370..3bf3f2aefe 100644 --- a/rust/crates/rusty-claude-cli/tests/compact_repl_panic.rs +++ b/rust/crates/rusty-claude-cli/tests/compact_repl_panic.rs @@ -1,3 +1,5 @@ +mod support; + use std::fs; use std::io::Write; use std::path::PathBuf; @@ -55,15 +57,15 @@ fn run_claw_repl( home: &std::path::Path, stdin: &str, ) -> Output { - let mut command = python_pty_command(env!("CARGO_BIN_EXE_claw")); + let mut command = python_terminal_command(env!("CARGO_BIN_EXE_claw")); let mut child = command .current_dir(cwd) .env_clear() + .envs(support::process_environment()) .env("ANTHROPIC_API_KEY", "test-compact-repl-key") .env("CLAW_CONFIG_HOME", config_home) .env("HOME", home) .env("NO_COLOR", "1") - .env("PATH", "/usr/bin:/bin") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -80,30 +82,12 @@ fn run_claw_repl( child.wait_with_output().expect("claw should finish") } -fn python_pty_command(claw: &str) -> Command { - let mut command = Command::new("python3"); - command.args([ - "-c", - r#" -import os -import pty -import subprocess -import sys - -claw = sys.argv[1] -payload = sys.stdin.buffer.read() -master, slave = pty.openpty() -child = subprocess.Popen([claw], stdin=slave, stdout=subprocess.PIPE, stderr=subprocess.PIPE) -os.close(slave) -os.write(master, payload) -stdout, stderr = child.communicate(timeout=30) -os.close(master) -sys.stdout.buffer.write(stdout) -sys.stderr.buffer.write(stderr) -raise SystemExit(child.returncode) -"#, - claw, - ]); +fn python_terminal_command(claw: &str) -> Command { + let mut command = Command::new(if cfg!(windows) { "python" } else { "python3" }); + command.arg( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/support/repl_console.py"), + ); + command.arg(claw); command } diff --git a/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs b/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs index edaf034955..96fe9425de 100644 --- a/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs +++ b/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs @@ -1,3 +1,5 @@ +mod support; + use std::collections::BTreeMap; use std::fs; use std::io::Write; @@ -312,12 +314,12 @@ fn run_case(case: ScenarioCase, workspace: &HarnessWorkspace, base_url: &str) -> command .current_dir(&workspace.root) .env_clear() + .envs(support::process_environment()) .env("ANTHROPIC_API_KEY", "test-parity-key") .env("ANTHROPIC_BASE_URL", base_url) .env("CLAW_CONFIG_HOME", &workspace.config_home) .env("HOME", &workspace.home) .env("NO_COLOR", "1") - .env("PATH", "/usr/bin:/bin") .args([ "--model", "sonnet", @@ -411,6 +413,33 @@ fn prepare_multi_tool_fixture(workspace: &HarnessWorkspace) { } fn prepare_plugin_fixture(workspace: &HarnessWorkspace) { + // A Windows `python` command can be an install-manager launcher whose + // bootstrap messages go to stdout. Resolve it before entering the isolated + // CLI environment, then execute the interpreter itself for the JSON tool. + let python = Command::new(if cfg!(windows) { "python" } else { "python3" }) + .args([ + "-I", + "-c", + "import json, sys; print(json.dumps(sys.executable))", + ]) + .output() + .expect("Python interpreter discovery should launch"); + assert!( + python.status.success(), + "Python interpreter discovery failed: {}", + String::from_utf8_lossy(&python.stderr) + ); + let discovery_stdout = String::from_utf8(python.stdout).expect("Python discovery UTF-8"); + let interpreter: String = serde_json::from_str( + discovery_stdout + .lines() + .last() + .expect("Python interpreter path"), + ) + .expect("Python discovery must end with its JSON-encoded executable path"); + assert!(Path::new(&interpreter).is_absolute()); + assert!(Path::new(&interpreter).is_file()); + let plugin_root = workspace .root .join("external-plugins") @@ -420,45 +449,36 @@ fn prepare_plugin_fixture(workspace: &HarnessWorkspace) { fs::create_dir_all(&tool_dir).expect("plugin tools dir"); fs::create_dir_all(&manifest_dir).expect("plugin manifest dir"); - let script_path = tool_dir.join("echo-json.sh"); + let script_path = tool_dir.join("echo-json.py"); fs::write( &script_path, - "#!/bin/sh\nINPUT=$(cat)\nprintf '{\"plugin\":\"%s\",\"tool\":\"%s\",\"input\":%s}\\n' \"$CLAWD_PLUGIN_ID\" \"$CLAWD_TOOL_NAME\" \"$INPUT\"\n", + r#"import json, os, sys +print(json.dumps({"plugin": os.environ["CLAWD_PLUGIN_ID"], "tool": os.environ["CLAWD_TOOL_NAME"], "input": json.load(sys.stdin)})) +"#, ) .expect("plugin script should write"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let mut permissions = fs::metadata(&script_path) - .expect("plugin script metadata") - .permissions(); - permissions.set_mode(0o755); - fs::set_permissions(&script_path, permissions).expect("plugin script should be executable"); - } fs::write( manifest_dir.join("plugin.json"), - r#"{ - "name": "parity-plugin", - "version": "1.0.0", - "description": "mock parity plugin", - "tools": [ - { - "name": "plugin_echo", - "description": "Echo JSON input", - "inputSchema": { - "type": "object", - "properties": { - "message": { "type": "string" } - }, - "required": ["message"], - "additionalProperties": false - }, - "command": "./tools/echo-json.sh", - "requiredPermission": "workspace-write" - } - ] -}"#, + json!({ + "name": "parity-plugin", + "version": "1.0.0", + "description": "mock parity plugin", + "tools": [{ + "name": "plugin_echo", + "description": "Echo JSON input", + "inputSchema": { + "type": "object", + "properties": {"message": {"type": "string"}}, + "required": ["message"], + "additionalProperties": false + }, + "command": interpreter, + "args": ["-I", "tools/echo-json.py"], + "requiredPermission": "workspace-write" + }] + }) + .to_string(), ) .expect("plugin manifest should write"); @@ -504,8 +524,16 @@ fn assert_read_file_roundtrip(workspace: &HarnessWorkspace, run: &ScenarioRun) { let output = run.response["tool_results"][0]["output"] .as_str() .expect("tool output"); - assert!(output.contains(&workspace.root.join("fixture.txt").display().to_string())); - assert!(output.contains("alpha parity line")); + let payload: Value = serde_json::from_str(output).expect("read_file output JSON"); + let actual_path = payload["file"]["filePath"].as_str().expect("filePath"); + assert_eq!( + fs::canonicalize(actual_path).expect("canonical reported file"), + fs::canonicalize(workspace.root.join("fixture.txt")).expect("canonical fixture file") + ); + assert!(payload["file"]["content"] + .as_str() + .expect("file content") + .contains("alpha parity line")); } fn assert_grep_chunk_assembly(_: &HarnessWorkspace, run: &ScenarioRun) { @@ -539,6 +567,7 @@ fn assert_write_file_allowed(workspace: &HarnessWorkspace, run: &ScenarioRun) { assert!(run.response["message"] .as_str() .expect("message text") + .replace('\\', "/") .contains("generated/output.txt")); let generated = workspace.root.join("generated").join("output.txt"); let contents = fs::read_to_string(&generated).expect("generated file should exist"); @@ -676,7 +705,13 @@ fn assert_plugin_tool_roundtrip(_: &HarnessWorkspace, run: &ScenarioRun) { let tool_output = run.response["tool_results"][0]["output"] .as_str() .expect("tool output"); - let parsed: Value = serde_json::from_str(tool_output).expect("plugin output json"); + assert_eq!( + run.response["tool_results"][0]["is_error"], + Value::Bool(false), + "plugin tool failed: {tool_output}" + ); + let parsed: Value = serde_json::from_str(tool_output) + .unwrap_or_else(|err| panic!("plugin output is not JSON: {err}; output: {tool_output:?}")); assert_eq!( parsed["plugin"], Value::String("parity-plugin@external".to_string()) diff --git a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs index a6c818db73..8b32f83e3b 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -638,13 +638,25 @@ fn global_cwd_flag_routes_status_workspace_and_short_alias_429() { &["--cwd", workspace_str, "--output-format", "json", "status"], ); assert_eq!(status["kind"], "status"); - assert_eq!(status["workspace"]["cwd"], expected_cwd); + assert_eq!( + fs::canonicalize(status["workspace"]["cwd"].as_str().expect("cwd")) + .expect("canonical cwd") + .display() + .to_string(), + expected_cwd + ); let short_status = assert_json_command( &launcher, &["-C", workspace_str, "status", "--output-format", "json"], ); - assert_eq!(short_status["workspace"]["cwd"], expected_cwd); + assert_eq!( + fs::canonicalize(short_status["workspace"]["cwd"].as_str().expect("cwd")) + .expect("canonical cwd") + .display() + .to_string(), + expected_cwd + ); let directory_status = assert_json_command( &launcher, @@ -655,7 +667,13 @@ fn global_cwd_flag_routes_status_workspace_and_short_alias_429() { "status", ], ); - assert_eq!(directory_status["workspace"]["cwd"], expected_cwd); + assert_eq!( + fs::canonicalize(directory_status["workspace"]["cwd"].as_str().expect("cwd")) + .expect("canonical cwd") + .display() + .to_string(), + expected_cwd + ); } #[test] @@ -2295,7 +2313,7 @@ fn config_json_attributes_precedence_and_shadowed_keys_425() { file["source"] == "project" && file["path"] .as_str() - .is_some_and(|path| path.ends_with(".claw/settings.json")) + .is_some_and(|path| std::path::Path::new(path).ends_with(".claw/settings.json")) }) .expect("project .claw/settings.json entry"); @@ -5320,7 +5338,7 @@ fn agents_create_scaffolds_toml_and_lists_locally_431() { assert_eq!(create_json["status"], "ok"); assert_eq!(create_json["format"], "toml"); assert_eq!( - reported_agent_path, + fs::canonicalize(&reported_agent_path).expect("canonical reported agent path"), fs::canonicalize(&agent_path).expect("canonical agent path") ); assert!(agent_path.is_file()); @@ -5337,7 +5355,8 @@ fn agents_create_scaffolds_toml_and_lists_locally_431() { .iter() .any(|agent| { agent["name"] == "my-agent" - && *agent["path"].as_str().expect("listed agent path") + && fs::canonicalize(agent["path"].as_str().expect("listed agent path")) + .expect("canonical reported list path") == fs::canonicalize(&agent_path).expect("canonical listed agent path") })); } diff --git a/rust/crates/rusty-claude-cli/tests/parallel_tool_result_pairing.rs b/rust/crates/rusty-claude-cli/tests/parallel_tool_result_pairing.rs index b4dfa42f91..6974fd2e7b 100644 --- a/rust/crates/rusty-claude-cli/tests/parallel_tool_result_pairing.rs +++ b/rust/crates/rusty-claude-cli/tests/parallel_tool_result_pairing.rs @@ -13,6 +13,8 @@ //! mock service does not validate pairing itself, which is why the assertion //! lives here. +mod support; + use std::fs; use std::path::PathBuf; use std::process::{Command, Output}; @@ -121,12 +123,12 @@ fn run_claw(workspace: &PathBuf, config_home: &PathBuf, home: &PathBuf, base_url Command::new(env!("CARGO_BIN_EXE_claw")) .current_dir(workspace) .env_clear() + .envs(support::process_environment()) .env("ANTHROPIC_API_KEY", "test-parallel-tool-key") .env("ANTHROPIC_BASE_URL", base_url) .env("CLAW_CONFIG_HOME", config_home) .env("HOME", home) .env("NO_COLOR", "1") - .env("PATH", "/usr/bin:/bin") .args([ "--model", "sonnet", diff --git a/rust/crates/rusty-claude-cli/tests/support/mod.rs b/rust/crates/rusty-claude-cli/tests/support/mod.rs new file mode 100644 index 0000000000..081ac47a6a --- /dev/null +++ b/rust/crates/rusty-claude-cli/tests/support/mod.rs @@ -0,0 +1,27 @@ +use std::ffi::OsString; + +// Keep credentials, proxy variables and user configuration out of subprocess +// tests, but preserve the operating-system environment needed to load DLLs, +// initialize networking and find the installed test interpreters on Windows. +pub fn process_environment() -> Vec<(&'static str, OsString)> { + #[cfg(windows)] + { + [ + "PATH", + "SYSTEMROOT", + "WINDIR", + "SYSTEMDRIVE", + "COMSPEC", + "PATHEXT", + "TEMP", + "TMP", + ] + .into_iter() + .filter_map(|name| std::env::var_os(name).map(|value| (name, value))) + .collect() + } + #[cfg(not(windows))] + { + vec![("PATH", OsString::from("/usr/bin:/bin"))] + } +} diff --git a/rust/crates/rusty-claude-cli/tests/support/repl_console.py b/rust/crates/rusty-claude-cli/tests/support/repl_console.py new file mode 100644 index 0000000000..426f96d30b --- /dev/null +++ b/rust/crates/rusty-claude-cli/tests/support/repl_console.py @@ -0,0 +1,92 @@ +"""Give the CLI terminal stdin and captured stdout, using only Python's stdlib.""" +import os +import subprocess +import sys + + +def run_unix(claw, payload): + import pty + master, slave = pty.openpty() + try: + with subprocess.Popen([claw], stdin=slave, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) as child: + os.close(slave) + slave = None + os.write(master, payload) + try: + stdout, stderr = child.communicate(timeout=30) + except subprocess.TimeoutExpired: + child.kill() + child.communicate() + raise + return child.returncode, stdout, stderr + finally: + if slave is not None: + os.close(slave) + os.close(master) + + +def run_windows(claw, payload): + import ctypes + import msvcrt + from ctypes import wintypes + + class KeyEvent(ctypes.Structure): + _fields_ = [("down", wintypes.BOOL), ("repeat", wintypes.WORD), + ("virtual_key", wintypes.WORD), ("scan", wintypes.WORD), + ("char", wintypes.WCHAR), ("control", wintypes.DWORD)] + + class Event(ctypes.Union): + _fields_ = [("key", KeyEvent), ("storage", ctypes.c_byte * 16)] + + class InputRecord(ctypes.Structure): + _fields_ = [("kind", wintypes.WORD), ("event", Event)] + + kernel = ctypes.WinDLL("kernel32", use_last_error=True) + kernel.AllocConsole.argtypes = [] + kernel.AllocConsole.restype = wintypes.BOOL + kernel.FreeConsole.argtypes = [] + kernel.FreeConsole.restype = wintypes.BOOL + kernel.WriteConsoleInputW.argtypes = [wintypes.HANDLE, ctypes.POINTER(InputRecord), + wintypes.DWORD, ctypes.POINTER(wintypes.DWORD)] + kernel.WriteConsoleInputW.restype = wintypes.BOOL + # This affects only this helper process, never the test runner's console. + kernel.FreeConsole() + if not kernel.AllocConsole(): + raise ctypes.WinError(ctypes.get_last_error()) + try: + with open("CONIN$", "r+b", buffering=0) as console: + records = [] + for char in payload.decode("utf-8").replace("\r\n", "\n").replace("\n", "\r"): + for down in (True, False): + record = InputRecord() + record.kind = 1 # KEY_EVENT + record.event.key = KeyEvent(down, 1, 13 if char == "\r" else 0, + 0, char, 0) + records.append(record) + inputs = (InputRecord * len(records))(*records) + written = wintypes.DWORD() + if not kernel.WriteConsoleInputW(msvcrt.get_osfhandle(console.fileno()), + inputs, len(inputs), ctypes.byref(written)): + raise ctypes.WinError(ctypes.get_last_error()) + if written.value != len(inputs): + raise RuntimeError("incomplete console input") + with subprocess.Popen([claw], stdin=console, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) as child: + try: + stdout, stderr = child.communicate(timeout=30) + except subprocess.TimeoutExpired: + child.kill() + child.communicate() + raise + return child.returncode, stdout, stderr + finally: + kernel.FreeConsole() + + +if __name__ == "__main__": + run = run_windows if os.name == "nt" else run_unix + code, stdout, stderr = run(sys.argv[1], sys.stdin.buffer.read()) + sys.stdout.buffer.write(stdout) + sys.stderr.buffer.write(stderr) + raise SystemExit(code) diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index a0d43df047..9a65a685e2 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -2728,7 +2728,7 @@ fn path_within_current_workspace(path: &str, allow_missing: bool) -> bool { '"' | '\'' | '`' | ',' | ';' | ')' | '(' | '[' | ']' | '{' | '}' ) }); - if looks_like_windows_absolute_path(trimmed) { + if !cfg!(windows) && looks_like_windows_absolute_path(trimmed) { return false; } @@ -6433,15 +6433,19 @@ fn execute_repl(input: ReplInput) -> Result { } struct ReplRuntime { - program: &'static str, + program: PathBuf, args: &'static [&'static str], } fn resolve_repl_runtime(language: &str) -> Result { match language.trim().to_ascii_lowercase().as_str() { "python" | "py" => Ok(ReplRuntime { - program: detect_first_command(&["python3", "python"]) - .ok_or_else(|| String::from("python runtime not found"))?, + program: detect_first_command(if cfg!(windows) { + &["python", "python3"] + } else { + &["python3", "python"] + }) + .ok_or_else(|| String::from("python runtime not found"))?, args: &["-c"], }), "javascript" | "js" | "node" => Ok(ReplRuntime { @@ -6458,11 +6462,8 @@ fn resolve_repl_runtime(language: &str) -> Result { } } -fn detect_first_command(commands: &[&'static str]) -> Option<&'static str> { - commands - .iter() - .copied() - .find(|command| command_exists(command)) +fn detect_first_command(commands: &[&str]) -> Option { + commands.iter().find_map(|command| resolve_command(command)) } #[derive(Clone, Copy)] @@ -6786,37 +6787,62 @@ fn execute_powershell(input: PowerShellInput) -> std::io::Result std::io::Result<&'static str> { - if command_exists("pwsh") { - Ok("pwsh") - } else if command_exists("powershell") { - Ok("powershell") - } else { - Err(std::io::Error::new( +fn detect_powershell_shell() -> std::io::Result { + detect_first_command(&["pwsh", "powershell"]).ok_or_else(|| { + std::io::Error::new( std::io::ErrorKind::NotFound, "PowerShell executable not found (expected `pwsh` or `powershell` in PATH)", - )) - } + ) + }) } -fn command_exists(command: &str) -> bool { - std::process::Command::new("sh") - .arg("-lc") - .arg(format!("command -v {command} >/dev/null 2>&1")) - .status() - .is_ok_and(|status| status.success()) +// Resolve the same executable that we will launch, without invoking a login +// shell (which can replace PATH). Windows also needs the selected extension: +// Command does not apply PATHEXT when given a bare program name. +fn resolve_command(command: &str) -> Option { + let path = std::env::var_os("PATH")?; + #[cfg(windows)] + let extensions = std::env::var("PATHEXT").unwrap_or_else(|_| ".COM;.EXE;.BAT;.CMD".to_string()); + for directory in std::env::split_paths(&path) { + let candidate = directory.join(command); + #[cfg(windows)] + { + if candidate.is_file() { + return Some(candidate); + } + for extension in extensions + .split(';') + .filter(|extension| !extension.is_empty()) + { + let candidate = directory.join(format!("{command}{extension}")); + if candidate.is_file() { + return Some(candidate); + } + } + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if std::fs::metadata(&candidate).is_ok_and(|metadata| { + metadata.is_file() && metadata.permissions().mode() & 0o111 != 0 + }) { + return Some(candidate); + } + } + } + None } #[allow(clippy::too_many_lines)] fn execute_shell_command( - shell: &str, + shell: &Path, command: &str, timeout: Option, run_in_background: Option, @@ -7352,7 +7378,7 @@ mod tests { fs::create_dir_all(&claw_dir).expect("create .claw dir"); // Use the actual OS temp dir so the worktree path matches the allowlist let tmp_root = std::env::temp_dir().to_str().expect("utf-8").to_string(); - let settings = format!("{{\"trustedRoots\": [\"{tmp_root}\"]}}"); + let settings = json!({"trustedRoots": [tmp_root]}).to_string(); fs::write(claw_dir.join("settings.json"), settings).expect("write settings"); // WorkerCreate with no per-call trusted_roots — config should supply them @@ -8603,6 +8629,7 @@ mod tests { assert!(output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with("/help/SKILL.md")); assert!(output["prompt"] .as_str() @@ -8622,6 +8649,7 @@ mod tests { assert!(dollar_output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with("/help/SKILL.md")); if let Some(home) = original_home { @@ -8661,6 +8689,7 @@ mod tests { assert!(skill_output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with(".claw/skills/plan/SKILL.md")); let command_result = execute_tool("Skill", &json!({ "skill": "/handoff" })) @@ -8670,6 +8699,7 @@ mod tests { assert!(command_output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with(".claw/commands/handoff.md")); std::env::set_current_dir(&original_dir).expect("restore cwd"); @@ -8708,6 +8738,7 @@ mod tests { assert!(output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with(".claude/skills/trace/SKILL.md")); assert_eq!(output["description"], "Project-local trace helper"); @@ -8770,11 +8801,13 @@ mod tests { assert!(omc_output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with(".omc/skills/hud/SKILL.md")); assert_eq!(omc_output["description"], "Project-local OMC HUD helper"); assert!(agents_output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with(".agents/skills/trace/SKILL.md")); assert_eq!( agents_output["description"], @@ -8830,6 +8863,7 @@ mod tests { assert!(output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with("skills/omc-learned/learned/SKILL.md")); assert_eq!(output["description"], "Learned OMC skill"); @@ -8889,6 +8923,7 @@ mod tests { assert!(direct_skill_output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with("skills/statusline/SKILL.md")); assert_eq!(direct_skill_output["description"], "Claude config skill"); @@ -8899,6 +8934,7 @@ mod tests { assert!(legacy_command_output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with("commands/doctor-check.md")); assert_eq!( legacy_command_output["description"], @@ -8956,6 +8992,7 @@ mod tests { assert!(output["path"] .as_str() .expect("path") + .replace('\\', "/") .ends_with(".claude/commands/team.md")); assert_eq!(output["description"], "Legacy team workflow"); @@ -10301,6 +10338,7 @@ mod tests { assert!(globbed_output["filenames"][0] .as_str() .expect("filename") + .replace('\\', "/") .ends_with("nested/lib.rs")); let glob_error = execute_tool("glob_search", &json!({ "pattern": "[" })) @@ -10753,7 +10791,14 @@ mod tests { .as_nanos() )); std::fs::create_dir_all(&dir).expect("create dir"); - let script = dir.join("pwsh"); + let script = dir.join(if cfg!(windows) { "pwsh.cmd" } else { "pwsh" }); + #[cfg(windows)] + std::fs::write( + &script, + "@echo off\r\n Date: Sun, 8 Nov 2026 19:01:39 +0000 Subject: [PATCH 2/3] test(plugins): make lifecycle fixtures Windows-compatible --- rust/crates/plugins/src/lib.rs | 42 ++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/rust/crates/plugins/src/lib.rs b/rust/crates/plugins/src/lib.rs index e566c6959a..49f52ac7c4 100644 --- a/rust/crates/plugins/src/lib.rs +++ b/rust/crates/plugins/src/lib.rs @@ -2502,20 +2502,42 @@ mod tests { fn write_lifecycle_plugin(root: &Path, name: &str, version: &str) -> PathBuf { let log_path = root.join("lifecycle.log"); + let extension = if cfg!(windows) { "cmd" } else { "sh" }; + let (init_script, shutdown_script) = if cfg!(windows) { + ( + "@echo off\r\necho init>>lifecycle.log\r\n", + "@echo off\r\necho shutdown>>lifecycle.log\r\n", + ) + } else { + ( + "#!/bin/sh\nprintf 'init\\n' >> lifecycle.log\n", + "#!/bin/sh\nprintf 'shutdown\\n' >> lifecycle.log\n", + ) + }; write_file( - root.join("lifecycle").join("init.sh").as_path(), - "#!/bin/sh\nprintf 'init\\n' >> lifecycle.log\n", + root.join("lifecycle") + .join(format!("init.{extension}")) + .as_path(), + init_script, ); write_file( - root.join("lifecycle").join("shutdown.sh").as_path(), - "#!/bin/sh\nprintf 'shutdown\\n' >> lifecycle.log\n", + root.join("lifecycle") + .join(format!("shutdown.{extension}")) + .as_path(), + shutdown_script, ); write_file( root.join(MANIFEST_RELATIVE_PATH).as_path(), - format!( - "{{\n \"name\": \"{name}\",\n \"version\": \"{version}\",\n \"description\": \"lifecycle plugin\",\n \"lifecycle\": {{\n \"Init\": [\"./lifecycle/init.sh\"],\n \"Shutdown\": [\"./lifecycle/shutdown.sh\"]\n }}\n}}" - ) - .as_str(), + &serde_json::json!({ + "name": name, + "version": version, + "description": "lifecycle plugin", + "lifecycle": { + "Init": [format!("./lifecycle/init.{extension}")], + "Shutdown": [format!("./lifecycle/shutdown.{extension}")] + } + }) + .to_string(), ); log_path } @@ -3626,7 +3648,7 @@ print(json.dumps({"plugin": os.environ["CLAWD_PLUGIN_ID"], "tool": os.environ["C registry.shutdown().expect("shutdown should succeed"); let log = fs::read_to_string(&log_path).expect("lifecycle log should exist"); - assert_eq!(log, "init\nshutdown\n"); + assert_eq!(log.replace("\r\n", "\n"), "init\nshutdown\n"); let _ = fs::remove_dir_all(config_home); let _ = fs::remove_dir_all(source_root); @@ -3823,7 +3845,7 @@ print(json.dumps({"plugin": os.environ["CLAWD_PLUGIN_ID"], "tool": os.environ["C if registry.initialize().is_ok() && registry.shutdown().is_ok() { // Verify lifecycle.log exists and has expected content if let Ok(log) = fs::read_to_string(&log_path) { - if log == "init\nshutdown\n" { + if log.replace("\r\n", "\n") == "init\nshutdown\n" { success_count.fetch_add(1, AtomicOrdering::Relaxed); } } From ba0e960711447ef26b5b01f42e5e2460b2f76300 Mon Sep 17 00:00:00 2001 From: clawdbot Date: Sun, 8 Nov 2026 19:39:18 +0000 Subject: [PATCH 3/3] fix(windows): address workspace test failures --- rust/Cargo.lock | 1 + rust/crates/runtime/Cargo.toml | 1 + rust/crates/runtime/src/file_ops.rs | 170 +++++++++++++++++- rust/crates/rusty-claude-cli/src/main.rs | 66 ++++--- .../tests/mock_parity_harness.rs | 10 +- 5 files changed, 213 insertions(+), 35 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index ab9412a0e3..38cb902a5f 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2142,6 +2142,7 @@ dependencies = [ "glob", "plugins", "regex", + "same-file", "serde", "serde_json", "sha2", diff --git a/rust/crates/runtime/Cargo.toml b/rust/crates/runtime/Cargo.toml index 1e6c16d3f2..8272cde0d7 100644 --- a/rust/crates/runtime/Cargo.toml +++ b/rust/crates/runtime/Cargo.toml @@ -13,6 +13,7 @@ plugins = { path = "../plugins" } regex = "1" serde = { version = "1", features = ["derive"] } serde_json.workspace = true +same-file = "1.0.6" telemetry = { path = "../telemetry" } tokio = { version = "1", features = ["io-std", "io-util", "macros", "process", "rt", "rt-multi-thread", "time"] } walkdir = "2" diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index f64e3665b3..5610c0b289 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -2,7 +2,7 @@ use std::cmp::Reverse; use std::collections::HashSet; use std::fs; use std::io; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; use std::time::Instant; use glob::Pattern; @@ -44,7 +44,7 @@ fn is_binary_file(path: &Path) -> io::Result { fn validate_workspace_boundary(resolved: &Path, workspace_root: &Path) -> io::Result<()> { let workspace_root = normalize_for_comparison(workspace_root); let comparable = normalize_for_comparison(resolved); - if comparable.starts_with(&workspace_root) || resolves_within(resolved, &workspace_root) { + if resolves_within(resolved, &workspace_root) { return Ok(()); } Err(io::Error::new( @@ -61,13 +61,87 @@ fn validate_workspace_boundary(resolved: &Path, workspace_root: &Path) -> io::Re /// filesystem resolves it. Windows accepts several spellings of one location /// that [`Path::starts_with`] treats as different: letter case (a working /// directory typed as `c:\users\...` for `C:\Users\...`), the `\\.\` device -/// namespace, and 8.3 short names. Only consulted after the lexical comparison -/// fails, so it can accept such a spelling but never widens what already -/// compared inside, and a path whose canonical form is outside stays rejected. +/// namespace, and 8.3 short names. Existing ancestors are canonicalized so +/// equivalent names are accepted while symlink and parent traversal escapes +/// remain outside the workspace. fn resolves_within(resolved: &Path, workspace_root: &Path) -> bool { - resolved - .canonicalize() - .is_ok_and(|canonical| normalize_for_comparison(&canonical).starts_with(workspace_root)) + if let Ok(canonical) = resolved.canonicalize() { + if normalize_for_comparison(&canonical).starts_with(workspace_root) { + return true; + } + } + + // Writes can target a path whose parent directories do not exist yet, so + // canonicalizing the whole path may not be possible. Resolve the nearest + // existing ancestor (including symlinks), then check its canonical path + // and file identity before applying only the missing suffix. + let mut existing_ancestor = resolved; + loop { + match fs::symlink_metadata(existing_ancestor) { + Ok(_) => break, + Err(error) if error.kind() == io::ErrorKind::NotFound => { + let Some(parent) = existing_ancestor.parent() else { + return false; + }; + existing_ancestor = parent; + } + Err(_) => return false, + } + } + + let Ok(canonical_ancestor) = existing_ancestor.canonicalize() else { + // In particular, do not treat a dangling symlink as a missing path. + return false; + }; + #[cfg(windows)] + if has_workspace_ancestor(&canonical_ancestor, workspace_root) { + return resolved + .strip_prefix(existing_ancestor) + .is_ok_and(|suffix| { + suffix.components().all(|component| { + !matches!( + component, + Component::ParentDir | Component::Prefix(_) | Component::RootDir + ) + }) + }); + } + + let Ok(missing_suffix) = resolved.strip_prefix(existing_ancestor) else { + return false; + }; + let mut canonical_path = normalize_for_comparison(&canonical_ancestor); + for component in missing_suffix.components() { + match component { + Component::CurDir => {} + // A parent component after a missing component could later + // traverse a symlink once the missing directory is created. + Component::ParentDir => return false, + Component::Normal(name) => canonical_path.push(name), + Component::Prefix(_) | Component::RootDir => return false, + } + } + + canonical_path.starts_with(workspace_root) +} + +/// Compare Windows directory identities when canonical paths use different +/// long/8.3 spellings. Path strings alone cannot establish this relationship. +#[cfg(windows)] +fn has_workspace_ancestor(path: &Path, workspace_root: &Path) -> bool { + let mut ancestor = path; + loop { + if same_file::is_same_file(ancestor, workspace_root).unwrap_or(false) { + return true; + } + let Some(parent) = ancestor.parent() else { + return false; + }; + if parent == ancestor { + return false; + } + ancestor = parent; + } } /// Text payload returned by file-reading operations. @@ -1131,7 +1205,7 @@ mod tests { use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; - #[cfg(unix)] + #[cfg(any(unix, windows))] use super::write_file_in_workspace; use super::{ component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search, @@ -1344,6 +1418,69 @@ mod tests { let _ = std::fs::remove_dir_all(&workspace); } + #[test] + #[cfg(windows)] + fn writes_inside_workspace_through_short_path_with_missing_nested_directories() { + let Some(local_app_data) = std::env::var_os("LOCALAPPDATA") else { + return; + }; + let long_temp = PathBuf::from(local_app_data).join("Temp"); + let short_temp = std::env::temp_dir(); + if !long_temp.is_dir() + || !short_temp.is_dir() + || long_temp == short_temp + || !same_file::is_same_file(&long_temp, &short_temp).unwrap_or(false) + { + // Some Windows hosts disable short-name generation or configure + // TEMP outside the user's local application data directory. + return; + } + + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("time should move forward") + .as_nanos(); + let workspace = long_temp.join(format!("clawd-native-short-path-{unique}")); + std::fs::create_dir_all(&workspace).expect("workspace dir should be created"); + let root = workspace + .canonicalize() + .expect("workspace should canonicalize"); + let short_workspace = short_temp.join( + workspace + .file_name() + .expect("workspace should have a file name"), + ); + let target = short_workspace.join("nested").join("demo.txt"); + + write_file_in_workspace(&target.to_string_lossy(), "safe content", &root) + .expect("an equivalent short path inside the workspace should be accepted"); + assert_eq!( + std::fs::read_to_string(workspace.join("nested/demo.txt")) + .expect("written file should be readable"), + "safe content" + ); + + let update = write_file_in_workspace(&target.to_string_lossy(), "updated content", &root) + .expect("an existing file through an equivalent short path should be accepted"); + assert_eq!(update.kind, "update"); + assert_eq!(update.original_file.as_deref(), Some("safe content")); + assert_eq!( + std::fs::read_to_string(workspace.join("nested/demo.txt")) + .expect("updated file should be readable"), + "updated content" + ); + + let escaping_target = short_workspace + .join("not-created") + .join("..") + .join("..") + .join("escape.txt"); + write_file_in_workspace(&escaping_target.to_string_lossy(), "must not escape", &root) + .expect_err("parent traversal from a missing path must stay outside the boundary"); + + let _ = std::fs::remove_dir_all(&workspace); + } + #[test] #[cfg(unix)] fn workspace_write_rejects_parent_symlink_escape_regression_3007_class() { @@ -1374,6 +1511,21 @@ mod tests { "write should not create through an escaping symlink" ); + let escaped_nested_child = link_dir.join("not-created").join("nested.txt"); + let result = write_file_in_workspace( + escaped_nested_child.to_string_lossy().as_ref(), + "must not escape", + &workspace, + ); + assert!( + result.is_err(), + "a missing descendant under an escaping symlink must be rejected" + ); + assert!( + !outside.join("not-created").exists(), + "write should not create missing directories through an escaping symlink" + ); + let _ = std::fs::remove_dir_all(&workspace); let _ = std::fs::remove_dir_all(&outside); } diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 02526affba..b001837d3b 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -15409,30 +15409,43 @@ mod tests { ) .expect("write hook"); } - if include_lifecycle { + let lifecycle = if include_lifecycle { fs::create_dir_all(root.join("lifecycle")).expect("lifecycle dir"); + let (extension, init_script, shutdown_script) = if cfg!(windows) { + ( + "cmd", + "@echo off\r\necho init>>lifecycle.log\r\n", + "@echo off\r\necho shutdown>>lifecycle.log\r\n", + ) + } else { + ( + "sh", + "#!/bin/sh\nprintf 'init\\n' >> lifecycle.log\n", + "#!/bin/sh\nprintf 'shutdown\\n' >> lifecycle.log\n", + ) + }; fs::write( - root.join("lifecycle").join("init.sh"), - "#!/bin/sh\nprintf 'init\\n' >> lifecycle.log\n", + root.join("lifecycle").join(format!("init.{extension}")), + init_script, ) .expect("write init lifecycle"); fs::write( - root.join("lifecycle").join("shutdown.sh"), - "#!/bin/sh\nprintf 'shutdown\\n' >> lifecycle.log\n", + root.join("lifecycle").join(format!("shutdown.{extension}")), + shutdown_script, ) .expect("write shutdown lifecycle"); - } + format!( + ",\n \"lifecycle\": {{\n \"Init\": [\"./lifecycle/init.{extension}\"],\n \"Shutdown\": [\"./lifecycle/shutdown.{extension}\"]\n }}" + ) + } else { + String::new() + }; let hooks = if include_hooks { ",\n \"hooks\": {\n \"PreToolUse\": [\"./hooks/pre.sh\"]\n }" } else { "" }; - let lifecycle = if include_lifecycle { - ",\n \"lifecycle\": {\n \"Init\": [\"./lifecycle/init.sh\"],\n \"Shutdown\": [\"./lifecycle/shutdown.sh\"]\n }" - } else { - "" - }; fs::write( root.join(".claude-plugin").join("plugin.json"), format!( @@ -16053,10 +16066,11 @@ mod tests { #[test] fn parses_system_prompt_options() { // given: system-prompt options for cwd and date + let cwd = std::env::temp_dir(); let args = vec![ "system-prompt".to_string(), "--cwd".to_string(), - "/tmp".to_string(), + cwd.to_string_lossy().into_owned(), "--date".to_string(), "2026-04-01".to_string(), ]; @@ -16068,7 +16082,7 @@ mod tests { assert_eq!( action, CliAction::PrintSystemPrompt { - cwd: PathBuf::from("/tmp"), + cwd, date: "2026-04-01".to_string(), model: DEFAULT_MODEL.to_string(), output_format: CliOutputFormat::Text, @@ -19349,16 +19363,22 @@ UU conflicted.rs", error.to_string().contains("session workspace mismatch"), "unexpected error: {error}" ); + let error = error.to_string(); + let workspace_a_name = workspace_a + .file_name() + .expect("workspace a should have a name") + .to_string_lossy(); + let workspace_b_name = workspace_b + .file_name() + .expect("workspace b should have a name") + .to_string_lossy(); + assert_ne!(workspace_a_name, workspace_b_name); assert!( - error - .to_string() - .contains(&workspace_b.display().to_string()), + error.contains(workspace_b_name.as_ref()), "expected current workspace in error: {error}" ); assert!( - error - .to_string() - .contains(&workspace_a.display().to_string()), + error.contains(workspace_a_name.as_ref()), "expected originating workspace in error: {error}" ); @@ -20535,7 +20555,9 @@ UU conflicted.rs", .expect("runtime should build"); assert_eq!( - fs::read_to_string(&log_path).expect("init log should exist"), + fs::read_to_string(&log_path) + .expect("init log should exist") + .replace("\r\n", "\n"), "init\n" ); @@ -20544,7 +20566,9 @@ UU conflicted.rs", .expect("plugin shutdown should succeed"); assert_eq!( - fs::read_to_string(&log_path).expect("shutdown log should exist"), + fs::read_to_string(&log_path) + .expect("shutdown log should exist") + .replace("\r\n", "\n"), "init\nshutdown\n" ); diff --git a/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs b/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs index 96fe9425de..53a5dcea11 100644 --- a/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs +++ b/rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs @@ -564,11 +564,11 @@ fn assert_write_file_allowed(workspace: &HarnessWorkspace, run: &ScenarioRun) { run.response["tool_uses"][0]["name"], Value::String("write_file".to_string()) ); - assert!(run.response["message"] - .as_str() - .expect("message text") - .replace('\\', "/") - .contains("generated/output.txt")); + let message = run.response["message"].as_str().expect("message text"); + assert!( + message.contains("write_file succeeded"), + "expected successful write confirmation in assistant message: {message:?}" + ); let generated = workspace.root.join("generated").join("output.txt"); let contents = fs::read_to_string(&generated).expect("generated file should exist"); assert_eq!(contents, "created by mock service\n");