From ac7fdcecf2b3687bd3414cf55534ea9d8d7aa5bf Mon Sep 17 00:00:00 2001 From: X-iZhang Date: Thu, 19 Mar 2026 18:38:57 +0000 Subject: [PATCH] refactor: improve readability of command validation and path extraction logic --- EvoScientist/backends.py | 31 ++++++++++++++++++------------- tests/test_backends.py | 14 +++++++++++--- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/EvoScientist/backends.py b/EvoScientist/backends.py index b05a4d8..7c23545 100644 --- a/EvoScientist/backends.py +++ b/EvoScientist/backends.py @@ -110,7 +110,11 @@ def _collect_executable_positions(command: str) -> set[int]: # First token is the executable itself — mark its offset offsets.add(seg_start) # pip install — mark the install-target token - if len(tokens) >= 3 and tokens[0] in ("pip", "pip3") and tokens[1] == "install": + if ( + len(tokens) >= 3 + and tokens[0] in ("pip", "pip3") + and tokens[1] == "install" + ): # Find position of the 3rd token (the package arg) onwards rest = pipe_seg_stripped for t in tokens[:2]: @@ -134,8 +138,8 @@ def _extract_all_paths(command: str) -> list[str]: # dashes, slashes. Looks inside quotes and unquoted tokens alike. # Excludes URL-like patterns (preceded by ://) path_re = re.compile( - r'(?)}\]]*)?)' # rest of the path ) for m in path_re.finditer(command): @@ -472,17 +476,9 @@ class CustomSandboxBackend(LocalShellBackend): Then delegates to LocalShellBackend.execute() for actual execution. """ - # Validate command safety - error = validate_command(command) - if error: - return ExecuteResponse( - output=error, - exit_code=1, - truncated=False, - ) - # Replace literal workspace-root absolute paths with ./ - # Catches cases where the agent uses the exact real path. + # Must happen BEFORE validation so workspace paths (e.g. /tmp/...) + # are sanitized before the system-path check fires. ws = str(self.cwd).rstrip("/") + "/" if ws in command: command = command.replace(ws, "./") @@ -494,6 +490,15 @@ class CustomSandboxBackend(LocalShellBackend): workspace_name=Path(str(self.cwd)).name, ) + # Validate command safety (after path sanitization) + error = validate_command(command) + if error: + return ExecuteResponse( + output=error, + exit_code=1, + truncated=False, + ) + # Delegate to parent for subprocess execution response = super().execute(command, timeout=timeout) diff --git a/tests/test_backends.py b/tests/test_backends.py index b36867f..c5495c6 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -395,12 +395,16 @@ class TestAbsolutePathDetection: def test_python_os_remove(self): """python -c with os.remove targeting system path.""" - result = validate_command("python -c \"import os; os.remove('/Users/foo/file')\"") + result = validate_command( + "python -c \"import os; os.remove('/Users/foo/file')\"" + ) assert result is not None assert "absolute system path" in result.lower() def test_python_shutil_rmtree(self): - result = validate_command("python -c \"import shutil; shutil.rmtree('/home/user/project')\"") + result = validate_command( + "python -c \"import shutil; shutil.rmtree('/home/user/project')\"" + ) assert result is not None assert "/home/" in result @@ -473,6 +477,7 @@ class TestAbsolutePathDetection: # dd itself is blocked by BLOCKED_COMMANDS, but the /dev path # should not trigger the absolute-path check due to = prefix from EvoScientist.backends import _extract_all_paths + assert _extract_all_paths("if=/dev/zero") == [] def test_safe_system_executable(self): @@ -494,7 +499,10 @@ class TestAbsolutePathDetection: assert validate_command("echo hello | /usr/bin/grep pattern") is None def test_safe_executable_in_chain(self): - assert validate_command("/usr/bin/python3 a.py && /opt/homebrew/bin/node b.js") is None + assert ( + validate_command("/usr/bin/python3 a.py && /opt/homebrew/bin/node b.js") + is None + ) def test_dangerous_second_arg_still_blocked(self): """System path as a non-executable argument should still be blocked."""