From 1dfcda4e3c195a03e51a3c69a88f18ebf525eeca Mon Sep 17 00:00:00 2001 From: helix4u <4317663+helix4u@users.noreply.github.com> Date: Thu, 23 Apr 2026 12:35:16 -0600 Subject: [PATCH] fix(approval): guard env and config overwrites --- run_agent.py | 1 + tests/run_agent/test_run_agent.py | 8 +++++ tests/tools/test_approval.py | 53 ++++++++++++++++++++++++++++++- tools/approval.py | 7 ++++ 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/run_agent.py b/run_agent.py index 63b0adb42..3a26fdead 100644 --- a/run_agent.py +++ b/run_agent.py @@ -262,6 +262,7 @@ _MAX_TOOL_WORKERS = 8 _DESTRUCTIVE_PATTERNS = re.compile( r"""(?:^|\s|&&|\|\||;|`)(?: rm\s|rmdir\s| + cp\s|install\s| mv\s| sed\s+-i| truncate\s| diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 991ca07d2..8d5e21f11 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -44,6 +44,14 @@ def _make_tool_defs(*names: str) -> list: ] +def test_is_destructive_command_treats_cp_as_mutating(): + assert run_agent._is_destructive_command("cp .env.local .env") is True + + +def test_is_destructive_command_treats_install_as_mutating(): + assert run_agent._is_destructive_command("install template.env .env") is True + + @pytest.fixture() def agent(): """Minimal AIAgent with mocked OpenAI client and tool loading.""" diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 2d7bfe6b0..a752b68d7 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -434,6 +434,58 @@ class TestSensitiveRedirectPattern: assert dangerous is False assert key is None + def test_redirect_to_local_dotenv_requires_approval(self): + dangerous, key, desc = detect_dangerous_command("echo TOKEN=x > .env") + assert dangerous is True + assert key is not None + assert "project env/config" in desc.lower() + + def test_redirect_to_nested_config_yaml_requires_approval(self): + dangerous, key, desc = detect_dangerous_command("echo mode: prod > deploy/config.yaml") + assert dangerous is True + assert key is not None + assert "project env/config" in desc.lower() + + def test_redirect_from_local_dotenv_source_is_safe(self): + dangerous, key, desc = detect_dangerous_command("cat .env > backup.txt") + assert dangerous is False + assert key is None + assert desc is None + + +class TestProjectSensitiveCopyPattern: + def test_cp_to_local_dotenv_requires_approval(self): + dangerous, key, desc = detect_dangerous_command("cp .env.local .env") + assert dangerous is True + assert key is not None + assert "project env/config" in desc.lower() + + def test_mv_to_nested_config_yaml_requires_approval(self): + dangerous, key, desc = detect_dangerous_command("mv tmp/generated.yaml config/config.yaml") + assert dangerous is True + assert key is not None + assert "project env/config" in desc.lower() + + def test_install_to_dotenv_requires_approval(self): + dangerous, key, desc = detect_dangerous_command("install -m 600 template.env .env.production") + assert dangerous is True + assert key is not None + assert "project env/config" in desc.lower() + + def test_cp_from_config_yaml_source_is_safe(self): + dangerous, key, desc = detect_dangerous_command("cp config.yaml backup.yaml") + assert dangerous is False + assert key is None + assert desc is None + + +class TestProjectSensitiveTeePattern: + def test_tee_to_local_dotenv_requires_approval(self): + dangerous, key, desc = detect_dangerous_command("printenv | tee .env.local") + assert dangerous is True + assert key is not None + assert "project env/config" in desc.lower() + class TestPatternKeyUniqueness: """Bug: pattern_key is derived by splitting on \\b and taking [1], so @@ -836,4 +888,3 @@ class TestChmodExecuteCombo: cmd = "chmod +x script.sh" dangerous, _, _ = detect_dangerous_command(cmd) assert dangerous is False - diff --git a/tools/approval.py b/tools/approval.py index fc344bd77..d88b3e877 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -63,11 +63,15 @@ _HERMES_ENV_PATH = ( r'(?:\$hermes_home|\$\{hermes_home\})/)' r'\.env\b' ) +_PROJECT_ENV_PATH = r'(?:(?:\.{1,2}/)?(?:[^\s/"\'`]+/)*\.env(?:\.[^/\s"\'`]+)*)' +_PROJECT_CONFIG_PATH = r'(?:(?:\.{1,2}/)?(?:[^\s/"\'`]+/)*config\.yaml)' _SENSITIVE_WRITE_TARGET = ( r'(?:/etc/|/dev/sd|' rf'{_SSH_SENSITIVE_PATH}|' rf'{_HERMES_ENV_PATH})' ) +_PROJECT_SENSITIVE_WRITE_TARGET = rf'(?:{_PROJECT_ENV_PATH}|{_PROJECT_CONFIG_PATH})' +_COMMAND_TAIL = r'(?:\s*(?:&&|\|\||;).*)?$' # ========================================================================= # Dangerous command patterns @@ -99,6 +103,8 @@ DANGEROUS_PATTERNS = [ (r'\b(bash|sh|zsh|ksh)\s+<\s*>?\s*["\']?{_SENSITIVE_WRITE_TARGET}', "overwrite system file via redirection"), + (rf'\btee\b.*["\']?{_PROJECT_SENSITIVE_WRITE_TARGET}["\']?{_COMMAND_TAIL}', "overwrite project env/config via tee"), + (rf'>>?\s*["\']?{_PROJECT_SENSITIVE_WRITE_TARGET}["\']?{_COMMAND_TAIL}', "overwrite project env/config via redirection"), (r'\bxargs\s+.*\brm\b', "xargs with rm"), (r'\bfind\b.*-exec\s+(/\S*/)?rm\b', "find -exec rm"), (r'\bfind\b.*-delete\b', "find -delete"), @@ -120,6 +126,7 @@ DANGEROUS_PATTERNS = [ (r'\bkill\b.*`\s*pgrep\b', "kill process via backtick pgrep expansion (self-termination)"), # File copy/move/edit into sensitive system paths (r'\b(cp|mv|install)\b.*\s/etc/', "copy/move file into /etc/"), + (rf'\b(cp|mv|install)\b.*\s["\']?{_PROJECT_SENSITIVE_WRITE_TARGET}["\']?{_COMMAND_TAIL}', "overwrite project env/config file"), (r'\bsed\s+-[^\s]*i.*\s/etc/', "in-place edit of system config"), (r'\bsed\s+--in-place\b.*\s/etc/', "in-place edit of system config (long flag)"), # Script execution via heredoc — bypasses the -e/-c flag patterns above.