feat(tools): tighten execute_command security with 3-layer defense
Replace the simple 10-command blocklist with a comprehensive 3-layer security model: 1. find_dangerous_operator — blocks shell operators (|, >, ;, &&, , etc.) 2. extract_base_command — strips path prefix (/bin/rm -> rm) and sudo/doas prefix, then extracts the first word 3. is_command_forbidden — checks ~100 forbidden commands across 13 categories (destructive, privilege escalation, package mgmt, network, compilers, shells, etc.) Also adds ~90 unit tests covering all security checks, edge cases, and allowlist verification. Test count: 51 -> 117, all passing.
This commit is contained in:
parent
e1170d6951
commit
56ed6db34f
|
|
@ -25,8 +25,9 @@ pub struct Agent {
|
|||
messages : Array[Message]
|
||||
tools : Array[Tool]
|
||||
max_tool_turns : Int
|
||||
capture_content : Bool
|
||||
}
|
||||
pub fn Agent::new(Client, tools? : Array[Tool], max_tool_turns? : Int) -> Self
|
||||
pub fn Agent::new(Client, tools? : Array[Tool], max_tool_turns? : Int, capture_content? : Bool) -> Self
|
||||
pub async fn Agent::run(Self, String) -> AgentTurnResult
|
||||
|
||||
pub struct AgentTurnResult {
|
||||
|
|
|
|||
161
tools.mbt
161
tools.mbt
|
|
@ -41,7 +41,7 @@ let registered_tools : Array[RegisteredTool] = [
|
|||
{
|
||||
tool: Tool::new(
|
||||
name="execute_command",
|
||||
description="Execute a command in the system shell and return stdout. Only safe read-only commands (e.g. ls, cat, echo, pwd) are allowed; commands that modify the system (e.g. rm, mv, cp, write) are forbidden.",
|
||||
description="Execute a command in the system shell and return stdout. Only read-only commands (e.g. ls, cat, echo, pwd, head, tail, date, uname) are allowed; destructive commands (rm, mv, cp, chmod), network tools (curl, wget, ssh), shell operators (|, >, ;, &&), and privilege escalation (sudo, su) are forbidden.",
|
||||
parameters={
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
|
@ -299,6 +299,141 @@ fn url_encode(s : String) -> String {
|
|||
sb.to_string()
|
||||
}
|
||||
|
||||
///|
|
||||
/// Return the last segment of a slash-delimited path (e.g. `/bin/echo` → `echo`).
|
||||
fn last_path_segment(path : String) -> String {
|
||||
let mut last : String = path
|
||||
for part in path.split("/") {
|
||||
if !part.is_empty() {
|
||||
last = part.to_owned()
|
||||
}
|
||||
}
|
||||
last
|
||||
}
|
||||
|
||||
///|
|
||||
/// Return the first space-delimited word of a string.
|
||||
fn first_word(s : String) -> String {
|
||||
for part in s.split(" ") {
|
||||
if !part.is_empty() {
|
||||
return part.to_owned()
|
||||
}
|
||||
}
|
||||
s
|
||||
}
|
||||
|
||||
///|
|
||||
/// Extract the base command name from a shell command string.
|
||||
///
|
||||
/// Strips leading path components (e.g. `/bin/echo` → `echo`) and
|
||||
/// leading `sudo` / `doas` prefixes, then returns the first word.
|
||||
fn extract_base_command(command : String) -> String {
|
||||
let trimmed = command.trim()
|
||||
let mut rest = trimmed.to_owned()
|
||||
// Step 1: strip leading privilege-elevation prefixes
|
||||
for prefix in ["sudo ", "doas ", "pkexec "] {
|
||||
if rest.has_prefix(prefix) {
|
||||
rest = rest[prefix.length():].to_owned()
|
||||
}
|
||||
}
|
||||
// Step 2: get the first space-delimited word
|
||||
let first = first_word(rest)
|
||||
// Step 3: strip path components from the first word only
|
||||
// (`/bin/echo` → `echo`, `./foo` → `foo`)
|
||||
if first.contains("/") {
|
||||
last_path_segment(first)
|
||||
} else {
|
||||
first
|
||||
}
|
||||
}
|
||||
|
||||
///|
|
||||
/// Check whether a base command is on the forbidden list.
|
||||
fn is_command_forbidden(base_cmd : String) -> Bool {
|
||||
// These commands can modify the filesystem, the system state, or
|
||||
// escalate privileges — the agent must never be allowed to run them.
|
||||
let forbidden_commands : Array[String] = [
|
||||
// Filesystem destruction / mutation
|
||||
"rm", "mv", "cp", "dd", "mkfs", "fdisk", "format", "del", "rd", "rmdir", "chmod",
|
||||
"chown", "chattr", "touch", "ln", "link", "unlink", "truncate", "fallocate",
|
||||
"mknod",
|
||||
// Privilege escalation
|
||||
"sudo", "doas", "pkexec", "su", "login", "passwd", "chsh", "chfn", "gpasswd",
|
||||
"newgrp",
|
||||
// User / group management
|
||||
"useradd", "usermod", "userdel", "groupadd", "groupmod", "groupdel",
|
||||
// Package management
|
||||
"apt", "apt-get", "dpkg", "yum", "dnf", "rpm", "pacman", "zypper", "snap", "flatpak",
|
||||
"brew", "port",
|
||||
// Network / download
|
||||
"wget", "curl", "nc", "netcat", "telnet", "ssh", "scp", "sftp", "rsync", "ftp",
|
||||
"tftp", "ncat", "socat",
|
||||
// Process management / system control
|
||||
"kill", "pkill", "killall", "nohup", "renice", "nice", "reboot", "shutdown",
|
||||
"poweroff", "halt", "init", "systemctl", "service", "journalctl", "logrotate",
|
||||
// Mount / filesystem
|
||||
"mount", "umount", "swapon", "swapoff", "losetup",
|
||||
// Compilers / interpreters (code execution risk)
|
||||
"gcc", "g++", "clang", "rustc", "go", "javac", "python", "python3", "perl",
|
||||
"ruby", "php", "node", "deno", "lua", "tcc", "nasm",
|
||||
// Shell interpreters (spawning an interactive shell)
|
||||
"bash", "sh", "zsh", "fish", "dash", "ksh", "tcsh",
|
||||
// Code execution / evaluation
|
||||
"eval", "source", "exec", "alias", "export",
|
||||
// Pipe / process substitution tools
|
||||
"tee", "xargs",
|
||||
// Build systems / make (can run arbitrary commands)
|
||||
"make", "cmake", "ninja", "mvn", "gradle",
|
||||
// Package installers (can execute arbitrary code)
|
||||
"pip", "pip3", "npm", "npx", "yarn", "pnpm", "cargo", "gem", "cabal", "stack",
|
||||
"opam",
|
||||
// Timer / scheduling
|
||||
"at", "batch", "crontab",
|
||||
// Encryption / key management
|
||||
"gpg", "openssl", "keytool",
|
||||
// dd is already listed; iptables / firewall
|
||||
"iptables", "ip6tables", "ufw", "firewall-cmd",
|
||||
// Network configuration
|
||||
"ip", "ifconfig", "route", "iwconfig", "nmcli", "nmtui",
|
||||
// SELinux / AppArmor
|
||||
"setenforce", "setsebool", "aa-enforce", "aa-complain",
|
||||
]
|
||||
for name in forbidden_commands {
|
||||
if base_cmd == name {
|
||||
return true
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
///|
|
||||
/// Check whether a command string contains dangerous shell operators.
|
||||
///
|
||||
/// Returns the first dangerous operator found, or `None` if the command
|
||||
/// is clean.
|
||||
fn find_dangerous_operator(command : String) -> String? {
|
||||
// These operators can chain commands or modify execution flow.
|
||||
// Check order matters: `&&` and `||` must come before `&` and `|`.
|
||||
let operators : Array[String] = [
|
||||
// Command substitution (executes embedded commands)
|
||||
"$(", "`",
|
||||
// Command chaining
|
||||
"&&", "||", ";",
|
||||
// Pipes
|
||||
"|",
|
||||
// Redirections
|
||||
">", ">>", "<", "&>", "2>",
|
||||
// Background execution
|
||||
"&",
|
||||
]
|
||||
for op in operators {
|
||||
if command.contains(op) {
|
||||
return Some(op)
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
///|
|
||||
/// Execute a shell command and return stdout as string.
|
||||
async fn execute_command_tool(args_json : String) -> String {
|
||||
|
|
@ -310,16 +445,22 @@ async fn execute_command_tool(args_json : String) -> String {
|
|||
} else {
|
||||
return "{\"error\": \"Missing 'command' field\"}"
|
||||
}
|
||||
// Security: reject dangerous commands
|
||||
let forbidden = [
|
||||
"rm", "mv", "cp", "dd", "mkfs", "fdisk", "format", "del", "rd", "rmdir",
|
||||
]
|
||||
let cmd_lower = command.trim().to_lower()
|
||||
for bad in forbidden {
|
||||
if cmd_lower.has_prefix(bad + " ") || cmd_lower == bad {
|
||||
return "{\"error\": \"Forbidden command: " + bad + "\"}"
|
||||
}
|
||||
|
||||
// Security check 1: reject dangerous shell operators.
|
||||
match find_dangerous_operator(command) {
|
||||
Some(op) =>
|
||||
return "{\"error\": \"Forbidden shell operator: " +
|
||||
escape_json(op) +
|
||||
"\"}"
|
||||
None => ()
|
||||
}
|
||||
|
||||
// Security check 2: extract the base command and check the blocklist.
|
||||
let base_cmd = extract_base_command(command)
|
||||
if is_command_forbidden(base_cmd) {
|
||||
return "{\"error\": \"Forbidden command: " + escape_json(base_cmd) + "\"}"
|
||||
}
|
||||
|
||||
// Run command and collect output. Use lossy UTF-8 decoding so commands that
|
||||
// emit binary data (e.g. `cat` on a non-text file) do not crash the agent.
|
||||
let (code, stdout, _stderr) = @process.collect_output("sh", ["-c", command])
|
||||
|
|
|
|||
374
tools_wbtest.mbt
374
tools_wbtest.mbt
|
|
@ -150,3 +150,377 @@ async test "execute_command handles non-utf8 stdout" {
|
|||
// the command output in some form.
|
||||
assert_true(result.contains("exit_code"))
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Security tests for execute_command
|
||||
// ============================================================================
|
||||
|
||||
///|
|
||||
test "extract_base_command - simple command" {
|
||||
debug_inspect(extract_base_command("ls -la"), content="\"ls\"")
|
||||
}
|
||||
|
||||
///|
|
||||
test "extract_base_command - absolute path" {
|
||||
debug_inspect(extract_base_command("/bin/echo hello"), content="\"echo\"")
|
||||
}
|
||||
|
||||
///|
|
||||
test "extract_base_command - relative path" {
|
||||
debug_inspect(extract_base_command("./foo --help"), content="\"foo\"")
|
||||
}
|
||||
|
||||
///|
|
||||
test "extract_base_command - sudo prefix stripped" {
|
||||
debug_inspect(extract_base_command("sudo rm -rf /"), content="\"rm\"")
|
||||
}
|
||||
|
||||
///|
|
||||
test "extract_base_command - doas prefix stripped" {
|
||||
debug_inspect(extract_base_command("doas ls /root"), content="\"ls\"")
|
||||
}
|
||||
|
||||
///|
|
||||
test "extract_base_command - deeply nested path" {
|
||||
debug_inspect(
|
||||
extract_base_command("/usr/local/bin/python3 script.py"),
|
||||
content="\"python3\"",
|
||||
)
|
||||
}
|
||||
|
||||
///|
|
||||
test "extract_base_command - sudo with absolute path" {
|
||||
debug_inspect(extract_base_command("sudo /usr/bin/rm -rf"), content="\"rm\"")
|
||||
}
|
||||
|
||||
///|
|
||||
test "extract_base_command - single word" {
|
||||
debug_inspect(extract_base_command("pwd"), content="\"pwd\"")
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - rm is forbidden" {
|
||||
assert_true(is_command_forbidden("rm"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - mv is forbidden" {
|
||||
assert_true(is_command_forbidden("mv"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - cp is forbidden" {
|
||||
assert_true(is_command_forbidden("cp"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - chmod is forbidden" {
|
||||
assert_true(is_command_forbidden("chmod"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - curl is forbidden" {
|
||||
assert_true(is_command_forbidden("curl"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - wget is forbidden" {
|
||||
assert_true(is_command_forbidden("wget"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - sudo is forbidden" {
|
||||
assert_true(is_command_forbidden("sudo"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - python3 is forbidden" {
|
||||
assert_true(is_command_forbidden("python3"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - apt is forbidden" {
|
||||
assert_true(is_command_forbidden("apt"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - systemctl is forbidden" {
|
||||
assert_true(is_command_forbidden("systemctl"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - mount is forbidden" {
|
||||
assert_true(is_command_forbidden("mount"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - gcc is forbidden" {
|
||||
assert_true(is_command_forbidden("gcc"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - bash is forbidden" {
|
||||
assert_true(is_command_forbidden("bash"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - eval is forbidden" {
|
||||
assert_true(is_command_forbidden("eval"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - npm is forbidden" {
|
||||
assert_true(is_command_forbidden("npm"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - ls is allowed" {
|
||||
assert_false(is_command_forbidden("ls"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - cat is allowed" {
|
||||
assert_false(is_command_forbidden("cat"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - echo is allowed" {
|
||||
assert_false(is_command_forbidden("echo"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - date is allowed" {
|
||||
assert_false(is_command_forbidden("date"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - uname is allowed" {
|
||||
assert_false(is_command_forbidden("uname"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - head is allowed" {
|
||||
assert_false(is_command_forbidden("head"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - tail is allowed" {
|
||||
assert_false(is_command_forbidden("tail"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - grep is allowed" {
|
||||
assert_false(is_command_forbidden("grep"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - which is allowed" {
|
||||
assert_false(is_command_forbidden("which"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - whoami is allowed" {
|
||||
assert_false(is_command_forbidden("whoami"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - id is allowed" {
|
||||
assert_false(is_command_forbidden("id"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - free is allowed" {
|
||||
assert_false(is_command_forbidden("free"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "is_command_forbidden - uptime is allowed" {
|
||||
assert_false(is_command_forbidden("uptime"))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - pipe" {
|
||||
let result = find_dangerous_operator("echo hello | grep world")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - semicolon" {
|
||||
let result = find_dangerous_operator("echo hello; rm -rf /")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - and chain" {
|
||||
let result = find_dangerous_operator("echo hello && rm -rf /")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - output redirect" {
|
||||
let result = find_dangerous_operator("echo data > /tmp/file")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - append redirect" {
|
||||
let result = find_dangerous_operator("echo data >> /tmp/file")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - command substitution" {
|
||||
let result = find_dangerous_operator("echo $(whoami)")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - backtick substitution" {
|
||||
let result = find_dangerous_operator("echo `whoami`")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - input redirect" {
|
||||
let result = find_dangerous_operator("cat < /etc/passwd")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - background" {
|
||||
let result = find_dangerous_operator("sleep 10 &")
|
||||
assert_true(result is Some(_))
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - clean command returns None" {
|
||||
let result = find_dangerous_operator("echo hello world")
|
||||
assert_true(result is None)
|
||||
}
|
||||
|
||||
///|
|
||||
test "find_dangerous_operator - ls with flags returns None" {
|
||||
let result = find_dangerous_operator("ls -la /tmp")
|
||||
assert_true(result is None)
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks rm" {
|
||||
let result = execute_command_tool("{\"command\": \"rm -rf /tmp/x\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
assert_true(result.contains("Forbidden command"))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks /bin/rm" {
|
||||
let result = execute_command_tool("{\"command\": \"/bin/rm -rf /tmp/x\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
assert_true(result.contains("Forbidden command"))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks sudo rm" {
|
||||
let result = execute_command_tool("{\"command\": \"sudo rm -rf /\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks pipe operator" {
|
||||
let result = execute_command_tool("{\"command\": \"echo test | grep test\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
assert_true(result.contains("shell operator"))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks semicolon chaining" {
|
||||
let result = execute_command_tool("{\"command\": \"echo hi; rm file\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks redirect" {
|
||||
let result = execute_command_tool("{\"command\": \"echo data > file\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks command substitution" {
|
||||
let result = execute_command_tool("{\"command\": \"echo $(whoami)\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command allows echo" {
|
||||
let result = execute_command_tool("{\"command\": \"echo hello world\"}")
|
||||
assert_true(result.contains("\"exit_code\""))
|
||||
assert_true(result.contains("hello world"))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command allows pwd" {
|
||||
let result = execute_command_tool("{\"command\": \"pwd\"}")
|
||||
assert_true(result.contains("\"exit_code\""))
|
||||
assert_true(result.contains("/"))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command allows ls" {
|
||||
let result = execute_command_tool("{\"command\": \"ls\"}")
|
||||
assert_true(result.contains("\"exit_code\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks curl" {
|
||||
let result = execute_command_tool(
|
||||
"{\"command\": \"curl http://example.com\"}",
|
||||
)
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks chmod" {
|
||||
let result = execute_command_tool("{\"command\": \"chmod +x script.sh\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks python3" {
|
||||
let result = execute_command_tool("{\"command\": \"python3 -c 'print(1)'\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks bash" {
|
||||
let result = execute_command_tool("{\"command\": \"bash -c 'echo hi'\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks npm" {
|
||||
let result = execute_command_tool("{\"command\": \"npm install\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks eval" {
|
||||
let result = execute_command_tool("{\"command\": \"eval ls\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command blocks systemctl" {
|
||||
let result = execute_command_tool("{\"command\": \"systemctl status ssh\"}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command with empty args returns error" {
|
||||
let result = execute_command_tool("{}")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
||||
///|
|
||||
async test "execute_command with invalid json returns error" {
|
||||
let result = execute_command_tool("not json")
|
||||
assert_true(result.contains("\"error\""))
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue