fix(shell): reject cmd.exe commands containing newlines (#11537)
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
This commit is contained in:
@@ -131,18 +131,24 @@ impl DeveloperClient {
|
||||
Some(false),
|
||||
Some(false),
|
||||
)),
|
||||
Tool::new(
|
||||
"shell".to_string(),
|
||||
format!(
|
||||
{
|
||||
let shell = shell_display_name();
|
||||
let newline_note = if shell == "cmd" {
|
||||
" Commands must be on a single line — cmd.exe silently truncates at the \
|
||||
first newline. Use `&` to chain (e.g. `echo a & echo b`) or set \
|
||||
GOOSE_SHELL=powershell for multi-line support."
|
||||
} else {
|
||||
""
|
||||
};
|
||||
let description = format!(
|
||||
"Execute a shell command in the current dir. Commands run under `{shell}` \
|
||||
(set GOOSE_SHELL to override) - write command strings in that shell's \
|
||||
syntax. Returns an object with stdout and stderr as separate fields. The \
|
||||
output of each stream is limited to up to 2000 lines, and longer outputs \
|
||||
will be saved to a temporary file.",
|
||||
shell = shell_display_name(),
|
||||
),
|
||||
Self::schema::<ShellParams>(),
|
||||
)
|
||||
syntax.{newline_note} Returns an object with stdout and stderr as separate \
|
||||
fields. The output of each stream is limited to up to 2000 lines, and \
|
||||
longer outputs will be saved to a temporary file.",
|
||||
);
|
||||
Tool::new("shell".to_string(), description, Self::schema::<ShellParams>())
|
||||
}
|
||||
.with_output_schema::<ShellOutput>()
|
||||
.annotate(ToolAnnotations::from_raw(
|
||||
Some("Shell".to_string()),
|
||||
|
||||
@@ -389,6 +389,16 @@ impl ShellTool {
|
||||
return Self::error_result("Command cannot be empty.", None);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
if shell_basename(&windows_shell()) == "cmd" && params.command.contains(['\n', '\r']) {
|
||||
return Self::error_result(
|
||||
"cmd.exe silently truncates commands at newlines — only the first line executes, \
|
||||
with exit code 0. Use `&` to chain commands on one line \
|
||||
(e.g. `echo a & echo b`), or set GOOSE_SHELL=powershell.",
|
||||
None,
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
let login_path = self.login_path.get().await;
|
||||
#[cfg(not(windows))]
|
||||
@@ -1272,6 +1282,29 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn cmd_rejects_newline_in_command() {
|
||||
for command in ["echo a\necho b", "echo a\r\necho b", "echo a\recho b"] {
|
||||
let tool = ShellTool::new_for_test().unwrap();
|
||||
let result = tool
|
||||
.shell(ShellParams {
|
||||
command: command.to_string(),
|
||||
timeout_secs: None,
|
||||
})
|
||||
.await;
|
||||
assert_eq!(
|
||||
result.is_error,
|
||||
Some(true),
|
||||
"expected error for {command:?}"
|
||||
);
|
||||
assert!(
|
||||
extract_text(&result).contains("cmd.exe"),
|
||||
"error should mention cmd.exe for {command:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn concurrent_calls_get_distinct_slots() {
|
||||
let tool = ShellTool::new_for_test().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user