🔒 security fix for PowerShell command injection#117
Conversation
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust tools crate's PowerShell execution path to pass commands via PowerShell's -EncodedCommand flag instead of -Command, aiming to change how script text is delivered to the shell.
Changes:
- Added the
base64dependency to thetoolscrate. - Converted PowerShell command strings to UTF-16LE bytes and Base64 before execution.
- Replaced
-Commandwith-EncodedCommandfor both foreground and background PowerShell invocations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
rust/crates/tools/src/lib.rs |
Encodes PowerShell command text and switches process invocation to -EncodedCommand. |
rust/crates/tools/Cargo.toml |
Adds the base64 crate needed for command encoding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .arg("-EncodedCommand") | ||
| .arg(encoded_command) |
| .arg("-EncodedCommand") | ||
| .arg(encoded_command); |
There was a problem hiding this comment.
Code Review
This pull request introduces Base64 encoding for shell commands to improve reliability when executing complex strings. It adds the base64 dependency and updates the execute_shell_command function to convert commands to UTF-16 LE before encoding them and passing them via the -EncodedCommand flag. A suggestion was made to pass a reference to the byte vector during encoding to follow idiomatic Rust practices and avoid potential ownership issues.
| .encode_utf16() | ||
| .flat_map(|u| u.to_le_bytes()) | ||
| .collect(); | ||
| let encoded_command = base64::engine::general_purpose::STANDARD.encode(utf16_bytes); |
There was a problem hiding this comment.
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.
| let encoded_command = base64::engine::general_purpose::STANDARD.encode(utf16_bytes); | |
| let encoded_command = base64::engine::general_purpose::STANDARD.encode(&utf16_bytes); |
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .arg("-EncodedCommand") | ||
| .arg(encoded_command); |
| assert_eq!( | ||
| output["stdout"], | ||
| "pwsh:VwByAGkAdABlAC0ATwB1AHQAcAB1AHQAIABoAGUAbABsAG8A" | ||
| ); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8da56aabb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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); |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
@codex, make changes based on:
Avoid expanding PowerShell payload past Windows argv limit
Switching from
-Commandto-EncodedCommandbase64-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 👍 / 👎.
There was a problem hiding this comment.
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
@gemini-code-assist, what are the steps and line numbers to make this fix? |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
🎯 What: Fixed a potential command injection vulnerability in PowerShell execution.
⚠️ Risk: Direct injection of user input into a shell execution via command arguments is a well-known vulnerability that could allow arbitrary command execution.
🛡️ Solution: Replaced the
-Commandflag with-EncodedCommand. The command string is now UTF-16LE and Base64 encoded before being passed to PowerShell, which is the recommended secure way to pass complex commands and avoid injection or quoting issues.PR created automatically by Jules for task 12627724688630408301 started by @badMade