refactor: improve readability of command validation and path extraction logic
This commit is contained in:
+18
-13
@@ -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 <path> — 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'(?<![:=/\w])' # not preceded by :, =, /, or word char (avoid URLs, env vars)
|
||||
r'(/(?:Users|home|tmp|var|etc|opt|usr|bin|sbin|dev|proc|sys|root)'
|
||||
r"(?<![:=/.\w])" # not preceded by :, =, /, ., or word char (avoid URLs, env vars, ./paths)
|
||||
r"(/(?:Users|home|tmp|var|etc|opt|usr|bin|sbin|dev|proc|sys|root)"
|
||||
r'(?:/[^\s\'",;|&<>)}\]]*)?)' # 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)
|
||||
|
||||
|
||||
+11
-3
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user