Skip to content

Commit 30c16ce

Browse files
committed
feat(shell_executor.py): add directory validation before command execution to handle directory permission issues
test(shell_executor.py): add unit test for command execution with restricted directory permissions to ensure proper error handling
1 parent dc19b6b commit 30c16ce

File tree

2 files changed

+31
-0
lines changed

2 files changed

+31
-0
lines changed

src/mcp_shell_server/shell_executor.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,18 @@ async def execute(
378378
start_time = time.time()
379379

380380
try:
381+
# Validate directory if specified
382+
try:
383+
self._validate_directory(directory)
384+
except ValueError as e:
385+
return {
386+
"error": str(e),
387+
"status": 1,
388+
"stdout": "",
389+
"stderr": str(e),
390+
"execution_time": time.time() - start_time,
391+
}
392+
381393
# Preprocess command to handle pipe operators
382394
preprocessed_command = self._preprocess_command(command)
383395
cleaned_command = self._clean_command(preprocessed_command)

tests/test_shell_executor.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,3 +388,22 @@ async def test_complex_pipeline_with_redirections(executor, temp_test_dir, monke
388388
assert len(lines) == 2
389389
assert lines[0].strip() == "HELLO"
390390
assert lines[1].strip() == "WORLD"
391+
392+
393+
@pytest.mark.asyncio
394+
async def test_directory_permissions(executor, temp_test_dir, monkeypatch):
395+
"""Test command execution with directory permission issues"""
396+
monkeypatch.setenv("ALLOW_COMMANDS", "ls")
397+
398+
# Create a directory with no execute permission
399+
no_exec_dir = os.path.join(temp_test_dir, "no_exec_dir")
400+
os.mkdir(no_exec_dir)
401+
os.chmod(no_exec_dir, 0o600) # Remove execute permission
402+
403+
try:
404+
result = await executor.execute(["ls"], directory=no_exec_dir)
405+
assert result["error"] == f"Directory is not accessible: {no_exec_dir}"
406+
assert result["status"] == 1
407+
finally:
408+
# Restore permissions for cleanup
409+
os.chmod(no_exec_dir, 0o700)

0 commit comments

Comments
 (0)