Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rust/crates/tools/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ publish.workspace = true

[dependencies]
api = { path = "../api" }
base64 = "0.22"
commands = { path = "../commands" }
flate2 = "1"
plugins = { path = "../plugins" }
Expand Down
50 changes: 37 additions & 13 deletions rust/crates/tools/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use base64::Engine;
use std::collections::{BTreeMap, BTreeSet};
use std::path::{Path, PathBuf};
use std::process::Command;
Expand Down Expand Up @@ -5164,12 +5165,18 @@ fn execute_shell_command(
timeout: Option<u64>,
run_in_background: Option<bool>,
) -> std::io::Result<runtime::BashCommandOutput> {
let utf16_bytes: Vec<u8> = command
.encode_utf16()
.flat_map(|u| u.to_le_bytes())
.collect();
let encoded_command = base64::engine::general_purpose::STANDARD.encode(utf16_bytes);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The encoded_command is used in two different code paths (background and foreground execution). Since base64::Engine::encode returns a String, passing it by value to .arg() in the background path (line 5179) would move it, making it unavailable for the foreground path (line 5208). Although the background path returns early, it is safer and more idiomatic to pass a reference to .arg() to avoid potential ownership issues if the code is refactored in the future.

Suggested change
let encoded_command = base64::engine::general_purpose::STANDARD.encode(utf16_bytes);
let encoded_command = base64::engine::general_purpose::STANDARD.encode(&utf16_bytes);

Comment on lines +5168 to +5172

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid expanding PowerShell payload past Windows argv limit

Switching from -Command to -EncodedCommand base64-expands every script (UTF-16LE + base64), which reduces the maximum runnable command size on Windows before process creation fails with command-line length errors. A command that previously fit as plain text can now exceed the CreateProcess argument limit once encoded, so large generated scripts will fail to execute even though they worked before this change.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex, make changes based on:

P2 Badge Avoid expanding PowerShell payload past Windows argv limit

Switching from -Command to -EncodedCommand base64-expands every script (UTF-16LE + base64), which reduces the maximum runnable command size on Windows before process creation fails with command-line length errors. A command that previously fit as plain text can now exceed the CreateProcess argument limit once encoded, so large generated scripts will fail to execute even though they worked before this change.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.


if run_in_background.unwrap_or(false) {
let child = std::process::Command::new(shell)
.arg("-NoProfile")
.arg("-NonInteractive")
.arg("-Command")
.arg(command)
.arg("-EncodedCommand")
.arg(encoded_command)
Comment on lines +5178 to +5179
.stdin(std::process::Stdio::null())
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
Expand Down Expand Up @@ -5197,8 +5204,8 @@ fn execute_shell_command(
process
.arg("-NoProfile")
.arg("-NonInteractive")
.arg("-Command")
.arg(command);
.arg("-EncodedCommand")
.arg(encoded_command);
Comment on lines +5207 to +5208
Comment on lines +5207 to +5208
process
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped());
Expand Down Expand Up @@ -6495,7 +6502,9 @@ mod tests {

#[test]
fn skill_loads_local_skill_prompt() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let home = temp_path("skills-home");
let skill_dir = home.join(".agents").join("skills").join("help");
fs::create_dir_all(&skill_dir).expect("skill dir should exist");
Expand Down Expand Up @@ -6552,7 +6561,9 @@ mod tests {

#[test]
fn skill_resolves_project_local_skills_and_legacy_commands() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let root = temp_path("project-skills");
let skill_dir = root.join(".claw").join("skills").join("plan");
let command_dir = root.join(".claw").join("commands");
Expand Down Expand Up @@ -6596,7 +6607,9 @@ mod tests {

#[test]
fn skill_loads_project_local_claude_skill_prompt() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let root = temp_path("project-skills");
let home = root.join("home");
let workspace = root.join("workspace");
Expand Down Expand Up @@ -6647,7 +6660,9 @@ mod tests {

#[test]
fn skill_loads_project_local_omc_and_agents_skill_prompts() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let root = temp_path("project-omc-skills");
let home = root.join("home");
let workspace = root.join("workspace");
Expand Down Expand Up @@ -6717,7 +6732,9 @@ mod tests {

#[test]
fn skill_loads_learned_skill_from_claude_config_dir() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let root = temp_path("claude-config-learned-skill");
let home = root.join("home");
let claude_config_dir = root.join("claude-config");
Expand Down Expand Up @@ -6772,7 +6789,9 @@ mod tests {

#[test]
fn skill_loads_direct_skill_and_legacy_command_from_claude_config_dir() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let root = temp_path("claude-config-direct-skill");
let home = root.join("home");
let claude_config_dir = root.join("claude-config");
Expand Down Expand Up @@ -6844,7 +6863,9 @@ mod tests {

#[test]
fn skill_loads_project_local_legacy_command_markdown() {
let _guard = env_lock().lock().expect("env lock should acquire");
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let root = temp_path("project-legacy-command");
let home = root.join("home");
let workspace = root.join("workspace");
Expand Down Expand Up @@ -8152,7 +8173,7 @@ mod tests {
std::fs::write(
&script,
r#"#!/bin/sh
while [ "$1" != "-Command" ] && [ $# -gt 0 ]; do shift; done
while [ "$1" != "-EncodedCommand" ] && [ $# -gt 0 ]; do shift; done
shift
printf 'pwsh:%s' "$1"
"#,
Expand Down Expand Up @@ -8182,7 +8203,10 @@ printf 'pwsh:%s' "$1"
let _ = std::fs::remove_dir_all(dir);

let output: serde_json::Value = serde_json::from_str(&result).expect("json");
assert_eq!(output["stdout"], "pwsh:Write-Output hello");
assert_eq!(
output["stdout"],
"pwsh:VwByAGkAdABlAC0ATwB1AHQAcAB1AHQAIABoAGUAbABsAG8A"
);
Comment on lines +8206 to +8209
assert!(output["stderr"].as_str().expect("stderr").is_empty());

let background_output: serde_json::Value = serde_json::from_str(&background).expect("json");
Expand Down
Loading