Skip to content

Commit a4a8a99

Browse files
committed
security hardening
1 parent 7bea815 commit a4a8a99

4 files changed

Lines changed: 107 additions & 2 deletions

File tree

src/handler/repl.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,12 @@ impl<'a> ReplHandler<'a> {
8989
break;
9090
}
9191
if self.shell_mode && prompt == "e" {
92-
shell_cmd::run(&last_completion)?;
92+
// Require a live confirmation on the controlling terminal before
93+
// executing. Reading from /dev/tty (not stdin) means a piped REPL
94+
// session cannot silently auto-execute a proposed command.
95+
if crate::tools::confirm_tty(&last_completion) {
96+
shell_cmd::run(&last_completion)?;
97+
}
9398
continue;
9499
}
95100
if self.shell_mode && prompt == "d" {

src/main.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,13 @@ fn shell_confirmation_loop(
216216
) -> Result<()> {
217217
use std::io::Write;
218218
loop {
219+
// The completion was streamed to the terminal by the handler and could
220+
// contain control bytes; reprint it sanitized next to the prompt so the
221+
// user always sees the true command they are about to run.
222+
println!(
223+
"Command: {}",
224+
render::sanitize_terminal_line(completion.trim())
225+
);
219226
print!("[E]xecute, [M]odify, [D]escribe, [A]bort: ");
220227
std::io::stdout().flush().ok();
221228
let mut answer = String::new();

src/render.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,22 @@
11
use std::io::{IsTerminal, Write};
22

3+
/// Escapes control bytes so model-controlled text cannot move the cursor or
4+
/// rewrite the screen to spoof what the user sees (e.g. an approval prompt).
5+
/// The result stays on a single line: ESC, CR, BS, newlines, tabs, and every
6+
/// other C0/C1 control are rendered literally as `\xNN`.
7+
pub fn sanitize_terminal_line(text: &str) -> String {
8+
let mut out = String::with_capacity(text.len());
9+
for ch in text.chars() {
10+
let is_control = (ch as u32) < 0x20 || ch as u32 == 0x7f || (0x80..=0x9f).contains(&(ch as u32));
11+
if is_control {
12+
out.push_str(&format!("\\x{:02x}", ch as u32));
13+
} else {
14+
out.push(ch);
15+
}
16+
}
17+
out
18+
}
19+
320
/// Minimal ANSI color support, mirroring the named colors shell_gpt exposes
421
/// via `typer.secho(fg=...)`. Markdown/live-region rendering is a later phase.
522
fn ansi_code(color: &str) -> &'static str {
@@ -60,3 +77,27 @@ impl TextPrinter {
6077
println!();
6178
}
6279
}
80+
81+
#[cfg(test)]
82+
mod tests {
83+
use super::sanitize_terminal_line;
84+
85+
#[test]
86+
fn escapes_cursor_control_sequences() {
87+
let malicious = "\x1b[2K\rrm -rf ~";
88+
let out = sanitize_terminal_line(malicious);
89+
assert_eq!(out, "\\x1b[2K\\x0drm -rf ~");
90+
assert!(!out.contains('\x1b'));
91+
assert!(!out.contains('\r'));
92+
}
93+
94+
#[test]
95+
fn escapes_newlines_and_tabs() {
96+
assert_eq!(sanitize_terminal_line("a\nb\tc"), "a\\x0ab\\x09c");
97+
}
98+
99+
#[test]
100+
fn ordinary_text_is_unchanged() {
101+
assert_eq!(sanitize_terminal_line("ls -la /tmp"), "ls -la /tmp");
102+
}
103+
}

src/tools.rs

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::io::Write;
33
use serde_json::json;
44

55
use crate::client::{ToolCall, ToolDefinition, ToolFunctionDefinition};
6+
use crate::render::sanitize_terminal_line;
67
use crate::shell_cmd;
78

89
pub const MAX_TOOL_ROUNDS: usize = 8;
@@ -41,13 +42,55 @@ fn execute_shell(call: &ToolCall, no_interaction: bool) -> String {
4142
}
4243

4344
fn confirm(command: &str) -> bool {
44-
print!("Model wants to execute `{command}`. Execute? [y/N] ");
45+
// The command is model-controlled; sanitize before display so control bytes
46+
// cannot rewrite the prompt and spoof what is being approved.
47+
print!(
48+
"Model wants to execute `{}`. Execute? [y/N] ",
49+
sanitize_terminal_line(command)
50+
);
4551
std::io::stdout().flush().ok();
4652
let mut answer = String::new();
4753
std::io::stdin().read_line(&mut answer).ok();
54+
is_yes(&answer)
55+
}
56+
57+
/// True only for an explicit yes; anything else (incl. empty/EOF) denies.
58+
pub fn is_yes(answer: &str) -> bool {
4859
matches!(answer.trim().to_ascii_lowercase().as_str(), "y" | "yes")
4960
}
5061

62+
/// Prompts on the controlling terminal (`/dev/tty`) — not stdin, which may be a
63+
/// pipe — to confirm executing a model-proposed `command`. Returns false,
64+
/// denying execution, when there is no controlling terminal to prompt on. The
65+
/// command is sanitized before display to prevent approval-prompt spoofing.
66+
pub fn confirm_tty(command: &str) -> bool {
67+
use std::io::{BufRead, BufReader};
68+
69+
let tty = match std::fs::OpenOptions::new()
70+
.read(true)
71+
.write(true)
72+
.open("/dev/tty")
73+
{
74+
Ok(tty) => tty,
75+
Err(_) => {
76+
eprintln!("Refusing to execute: no controlling terminal available to confirm.");
77+
return false;
78+
}
79+
};
80+
let mut writer = &tty;
81+
let _ = write!(
82+
writer,
83+
"Execute `{}`? [y/N] ",
84+
sanitize_terminal_line(command)
85+
);
86+
let _ = writer.flush();
87+
let mut answer = String::new();
88+
if BufReader::new(&tty).read_line(&mut answer).is_err() {
89+
return false;
90+
}
91+
is_yes(&answer)
92+
}
93+
5194
#[cfg(test)]
5295
mod tests {
5396
use super::*;
@@ -64,4 +107,13 @@ mod tests {
64107
fn registry_exposes_the_shell_tool() {
65108
assert_eq!(definitions()[0].function.name, "execute_shell_command");
66109
}
110+
#[test]
111+
fn confirmation_denies_by_default() {
112+
for yes in ["y", "yes", "Y", "YES", " yes \n"] {
113+
assert!(is_yes(yes), "{yes:?} should confirm");
114+
}
115+
for no in ["", "\n", "n", "no", "sure", "yep", "yolo"] {
116+
assert!(!is_yes(no), "{no:?} should deny");
117+
}
118+
}
67119
}

0 commit comments

Comments
 (0)