Skip to content
Merged
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
7 changes: 4 additions & 3 deletions src/goose/toolkit/developer.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from pathlib import Path
from subprocess import CompletedProcess, run
from typing import List
from goose.utils.check_shell_command import is_dangerous_command

from exchange import Message
from rich import box
Expand Down Expand Up @@ -135,7 +136,7 @@ def shell(self, command: str) -> str:
command (str): The shell command to run. It can support multiline statements
if you need to run more than one at a time
"""
self.notifier.status("running shell command")
self.notifier.status("planning to run shell command")
# Log the command being executed in a visually structured format (Markdown).
# The `.log` method is used here to log the command execution in the application's UX
# this method is dynamically attached to functions in the Goose framework to handle user-visible
Expand All @@ -156,13 +157,13 @@ def shell(self, command: str) -> str:
rating = int(rating)
except ValueError:
rating = 5 # if we can't interpret we default to unsafe
if int(rating) > 3:
if is_dangerous_command(command) or int(rating) > 3:
if not keep_unsafe_command_prompt(command):
raise RuntimeError(
f"The command {command} was rejected as dangerous by the user."
+ " Do not proceed further, instead ask for instructions."
)

self.notifier.status("running shell command")
result: CompletedProcess = run(command, shell=True, text=True, capture_output=True, check=False)
if result.returncode == 0:
output = "Command succeeded"
Expand Down
33 changes: 33 additions & 0 deletions src/goose/utils/check_shell_command.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import re


def is_dangerous_command(command: str) -> bool:
"""
Check if the command matches any dangerous patterns.

Args:
command (str): The shell command to check.

Returns:
bool: True if the command is dangerous, False otherwise.
"""
dangerous_patterns = [
r"\brm\b", # rm command
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: I think rm on a single file might be fine, but I am scared of rm -r - especially combined with the file path checks below. Wdyt?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

better safe than sorry.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A bit late, but I think it'd be nice to add some user flexibility here, possibly via config setting somewhere. eg: one might also not want to allow curl, http, etc.

r"\bgit\s+push\b", # git push command
r"\bsudo\b", # sudo command
# Add more dangerous command patterns here
r"\bmv\b", # mv command
r"\bchmod\b", # chmod command
r"\bchown\b", # chown command
r"\bmkfs\b", # mkfs command
r"\bsystemctl\b", # systemctl command
r"\breboot\b", # reboot command
r"\bshutdown\b", # shutdown command
# Manipulating files in ~/ directly or dot files
r"^~/[^/]+$", # Files directly in home directory
r"/\.[^/]+$", # Dot files
]
for pattern in dangerous_patterns:
if re.search(pattern, command):
return True
return False
27 changes: 27 additions & 0 deletions tests/utils/test_check_shell_command.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import pytest
from goose.utils.check_shell_command import is_dangerous_command


@pytest.mark.parametrize(
"command",
[
"rm -rf /",
"git push origin master",
"sudo reboot",
"mv /etc/passwd /tmp/",
"chmod 777 /etc/passwd",
"chown root:root /etc/passwd",
"mkfs -t ext4 /dev/sda1",
"systemctl stop nginx",
"reboot",
"shutdown now",
"echo hello > ~/.bashrc",
],
)
def test_dangerous_commands(command):
assert is_dangerous_command(command)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: probably want to assert some of the inverse, with safe commands too

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

that's what the second test does.



@pytest.mark.parametrize("command", ["ls -la", 'echo "Hello World"', "cp ~/folder/file.txt /tmp/"])
def test_safe_commands(command):
assert not is_dangerous_command(command)