From 3c8ec7037c6aa59d04faf9c2ef5a1fee02c6fb26 Mon Sep 17 00:00:00 2001 From: konsisumer Date: Thu, 9 Apr 2026 07:23:34 +0200 Subject: [PATCH] fix(agent): catch PermissionError in subdirectory hint discovery Wrap is_dir() in _is_valid_subdir() and is_file() in _load_hints_for_directory() with OSError handlers so that inaccessible directories (e.g. /root from a non-root Daytona host user) are silently skipped instead of crashing the agent. The existing PermissionError PRs for prompt_builder.py (#6247, #6321, #6355) do not cover subdirectory_hints.py, which was identified as a separate crash path in the #6214 comments. Ref: #6214 --- agent/subdirectory_hints.py | 10 ++++-- tests/agent/test_subdirectory_hints.py | 43 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/agent/subdirectory_hints.py b/agent/subdirectory_hints.py index 96903e2e2..dcc514b90 100644 --- a/agent/subdirectory_hints.py +++ b/agent/subdirectory_hints.py @@ -159,7 +159,10 @@ class SubdirectoryHintTracker: def _is_valid_subdir(self, path: Path) -> bool: """Check if path is a valid directory to scan for hints.""" - if not path.is_dir(): + try: + if not path.is_dir(): + return False + except OSError: return False if path in self._loaded_dirs: return False @@ -172,7 +175,10 @@ class SubdirectoryHintTracker: found_hints = [] for filename in _HINT_FILENAMES: hint_path = directory / filename - if not hint_path.is_file(): + try: + if not hint_path.is_file(): + continue + except OSError: continue try: content = hint_path.read_text(encoding="utf-8").strip() diff --git a/tests/agent/test_subdirectory_hints.py b/tests/agent/test_subdirectory_hints.py index 7d2bc607c..7c1a74e66 100644 --- a/tests/agent/test_subdirectory_hints.py +++ b/tests/agent/test_subdirectory_hints.py @@ -3,6 +3,7 @@ import os import pytest from pathlib import Path +from unittest.mock import patch from agent.subdirectory_hints import SubdirectoryHintTracker @@ -189,3 +190,45 @@ class TestSubdirectoryHintTracker: "terminal", {"command": "curl https://example.com/frontend/api"} ) assert result is None + + +class TestPermissionErrorHandling: + """Regression tests for PermissionError in filesystem checks (ref #6214).""" + + def test_is_valid_subdir_permission_error(self, tmp_path): + """_is_valid_subdir should return False when is_dir() raises PermissionError.""" + tracker = SubdirectoryHintTracker(working_dir=str(tmp_path)) + restricted = tmp_path / "restricted" + restricted.mkdir() + with patch.object(Path, "is_dir", side_effect=PermissionError("Permission denied")): + assert tracker._is_valid_subdir(restricted) is False + + def test_load_hints_permission_error_on_is_file(self, tmp_path): + """_load_hints_for_directory should skip files when is_file() raises PermissionError.""" + tracker = SubdirectoryHintTracker(working_dir=str(tmp_path)) + restricted = tmp_path / "restricted" + restricted.mkdir() + original_is_file = Path.is_file + def patched_is_file(self): + if "restricted" in str(self): + raise PermissionError("Permission denied") + return original_is_file(self) + with patch.object(Path, "is_file", patched_is_file): + result = tracker._load_hints_for_directory(restricted) + assert result is None + + def test_check_tool_call_survives_inaccessible_path(self, project): + """Full check_tool_call should not crash when a path is inaccessible.""" + tracker = SubdirectoryHintTracker(working_dir=str(project)) + original_is_dir = Path.is_dir + def patched_is_dir(self): + if "backend" in str(self) and "src" not in str(self): + raise PermissionError("Permission denied") + return original_is_dir(self) + with patch.object(Path, "is_dir", patched_is_dir): + # Should not raise — gracefully skip the inaccessible directory + result = tracker.check_tool_call( + "read_file", {"path": str(project / "backend" / "src" / "main.py")} + ) + # Result may be None (backend skipped) — the key point is no crash + assert result is None or isinstance(result, str)