Streamline / clarify shell command control configuration (#4628)

* Streamline / clarify shell command control configuration

* Fix lint

.env.template

@@ -41,14 +41,6 @@
 ## For example, to disable coding related features, uncomment the next line
 # DISABLED_COMMAND_CATEGORIES=autogpt.commands.analyze_code,autogpt.commands.execute_code,autogpt.commands.git_operations,autogpt.commands.improve_code,autogpt.commands.write_tests
 
-## DENY_COMMANDS - The list of commands that are not allowed to be executed by Auto-GPT (Default: None)
-# the following are examples:
-# DENY_COMMANDS=cd,nano,vim,vi,emacs,rm,sudo,top,ping,ssh,scp
-
-## ALLOW_COMMANDS - ONLY those commands will be allowed to be executed by Auto-GPT
-# the following are examples:
-# ALLOW_COMMANDS=ls,git,cat,grep,find,echo,ps,curl,wget
-
 
 ################################################################################
 ### LLM PROVIDER
@@ -89,6 +81,21 @@ OPENAI_API_KEY=your-openai-api-key
 ## EMBEDDING_MODEL       - Model to use for creating embeddings
 # EMBEDDING_MODEL=text-embedding-ada-002
 
+################################################################################
+### SHELL EXECUTION
+################################################################################
+
+## SHELL_COMMAND_CONTROL - Whether to use "allowlist" or "denylist" to determine what shell commands can be executed (Default: denylist)
+# SHELL_COMMAND_CONTROL=denylist
+
+## ONLY if SHELL_COMMAND_CONTROL is set to denylist:
+## SHELL_DENYLIST - List of shell commands that ARE NOT allowed to be executed by Auto-GPT (Default: sudo,su)
+# SHELL_DENYLIST=sudo,su
+
+## ONLY if SHELL_COMMAND_CONTROL is set to allowlist:
+## SHELL_ALLOWLIST - List of shell commands that ARE allowed to be executed by Auto-GPT (Default: None)
+# SHELL_ALLOWLIST=
+
 ################################################################################
 ### MEMORY
 ################################################################################

autogpt/commands/execute_code.py

@@ -13,6 +13,9 @@ from autogpt.logs import logger
 from autogpt.setup import CFG
 from autogpt.workspace.workspace import Workspace
 
+ALLOWLIST_CONTROL = "allowlist"
+DENYLIST_CONTROL = "denylist"
+
 
 @command(
     "execute_python_code",
@@ -152,21 +155,15 @@ def validate_command(command: str, config: Config) -> bool:
     Returns:
         bool: True if the command is allowed, False otherwise
     """
-    tokens = command.split()
-
-    if not tokens:
+    if not command:
         return False
 
-    if config.deny_commands and tokens[0] in config.deny_commands:
-        return False
+    command_name = command.split()[0]
 
-    for keyword in config.allow_commands:
-        if keyword in tokens:
-            return True
-    if config.allow_commands:
-        return False
-
-    return True
+    if config.shell_command_control == ALLOWLIST_CONTROL:
+        return command_name in config.shell_allowlist
+    else:
+        return command_name not in config.shell_denylist
 
 
 @command(

autogpt/config/config.py

@@ -38,17 +38,21 @@ class Config(metaclass=Singleton):
         else:
             self.disabled_command_categories = []
 
-        deny_commands = os.getenv("DENY_COMMANDS")
-        if deny_commands:
-            self.deny_commands = deny_commands.split(",")
-        else:
-            self.deny_commands = []
+        self.shell_command_control = os.getenv("SHELL_COMMAND_CONTROL", "denylist")
 
-        allow_commands = os.getenv("ALLOW_COMMANDS")
-        if allow_commands:
-            self.allow_commands = allow_commands.split(",")
+        # DENY_COMMANDS is deprecated and included for backwards-compatibility
+        shell_denylist = os.getenv("SHELL_DENYLIST", os.getenv("DENY_COMMANDS"))
+        if shell_denylist:
+            self.shell_denylist = shell_denylist.split(",")
         else:
-            self.allow_commands = []
+            self.shell_denylist = ["sudo", "su"]
+
+        # ALLOW_COMMANDS is deprecated and included for backwards-compatibility
+        shell_allowlist = os.getenv("SHELL_ALLOWLIST", os.getenv("ALLOW_COMMANDS"))
+        if shell_allowlist:
+            self.shell_allowlist = shell_allowlist.split(",")
+        else:
+            self.shell_allowlist = []
 
         self.ai_settings_file = os.getenv("AI_SETTINGS_FILE", "ai_settings.yaml")
         self.prompt_settings_file = os.getenv(

tests/integration/test_execute_code.py

@@ -5,18 +5,12 @@ import tempfile
 from typing import Callable
 
 import pytest
-from pytest_mock import MockerFixture
 
 import autogpt.commands.execute_code as sut  # system under testing
 from autogpt.config import Config
 from autogpt.config.ai_config import AIConfig
 
 
-@pytest.fixture
-def config_allow_execute(config: Config, mocker: MockerFixture) -> Callable:
-    yield mocker.patch.object(config, "execute_local_commands", True)
-
-
 @pytest.fixture
 def random_code(random_string) -> Callable:
     return f"print('Hello {random_string}!')"
@@ -92,14 +86,43 @@ def test_execute_python_file_not_found(config: Config):
     )
 
 
-def test_execute_shell(config_allow_execute: bool, random_string: str, config: Config):
+def test_execute_shell(random_string: str, config: Config):
     result = sut.execute_shell(f"echo 'Hello {random_string}!'", config)
     assert f"Hello {random_string}!" in result
 
 
-def test_execute_shell_deny_command(
-    python_test_file: str, config_allow_execute: bool, config: Config
-):
-    config.deny_commands = ["echo"]
+def test_execute_shell_local_commands_not_allowed(random_string: str, config: Config):
+    result = sut.execute_shell(f"echo 'Hello {random_string}!'", config)
+    assert f"Hello {random_string}!" in result
+
+
+def test_execute_shell_denylist_should_deny(config: Config, random_string: str):
+    config.shell_denylist = ["echo"]
+
     result = sut.execute_shell(f"echo 'Hello {random_string}!'", config)
     assert "Error:" in result and "not allowed" in result
+
+
+def test_execute_shell_denylist_should_allow(config: Config, random_string: str):
+    config.shell_denylist = ["cat"]
+
+    result = sut.execute_shell(f"echo 'Hello {random_string}!'", config)
+    assert "Hello" in result and random_string in result
+    assert "Error" not in result
+
+
+def test_execute_shell_allowlist_should_deny(config: Config, random_string: str):
+    config.shell_command_control = sut.ALLOWLIST_CONTROL
+    config.shell_allowlist = ["cat"]
+
+    result = sut.execute_shell(f"echo 'Hello {random_string}!'", config)
+    assert "Error:" in result and "not allowed" in result
+
+
+def test_execute_shell_allowlist_should_allow(config: Config, random_string: str):
+    config.shell_command_control = sut.ALLOWLIST_CONTROL
+    config.shell_allowlist = ["echo"]
+
+    result = sut.execute_shell(f"echo 'Hello {random_string}!'", config)
+    assert "Hello" in result and random_string in result
+    assert "Error" not in result