From 27be335554779d57ef19823e7f20ebbe8b081cac Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Thu, 27 Aug 2026 11:06:50 +1000 Subject: [PATCH] fix(input-handler): traverse ancestors with O_PATH where available The descriptor-relative open walks every component from the filesystem root with O_RDONLY | O_DIRECTORY | O_NOFOLLOW. O_RDONLY demands read access on each ancestor, including "/", but traversal only needs search access. Sandboxes that grant access per path hierarchy -- Landlock, systemd ProtectSystem, containers with restricted mounts -- do not grant read on "/", so the very first open fails with EACCES and every scan reports "Could not safely open file". Use O_PATH for the intermediate directory descriptors where it exists. It requires only search access, and the resulting descriptor is still valid for the relative openat() calls this walk performs. Elsewhere (macOS, Windows) getattr falls back to O_RDONLY, i.e. 0, leaving the flags unchanged. The symlink guarantees are unaffected: O_DIRECTORY still rejects a symlinked intermediate component with ENOTDIR, and the final component is still opened with O_RDONLY | O_NOFOLLOW, so a symlinked target still fails with ELOOP. Both errnos already map to _UnsafeFileError. Signed-off-by: Bevan Kay --- src/skillspector/input_handler.py | 6 +++++- tests/unit/test_input_handler.py | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/skillspector/input_handler.py b/src/skillspector/input_handler.py index 9daf9e18..6468bbf3 100644 --- a/src/skillspector/input_handler.py +++ b/src/skillspector/input_handler.py @@ -297,7 +297,11 @@ def _open_regular_file_no_follow(file_path: Path) -> BinaryIO: def _open_regular_file_from_trusted_directory(file_path: Path) -> BinaryIO: """Open *file_path* one non-symlinked component at a time.""" - directory_flags = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW + # Traversal only needs search access on each component. Prefer O_PATH where it + # exists (Linux); O_RDONLY additionally demands read access, which sandboxes + # such as Landlock withhold on "/". Elsewhere (e.g. macOS) this is O_RDONLY, + # i.e. 0, leaving the flags unchanged. + directory_flags = os.O_DIRECTORY | os.O_NOFOLLOW | getattr(os, "O_PATH", os.O_RDONLY) directory_fd: int | None = None try: directory_fd = os.open(file_path.anchor, directory_flags) diff --git a/tests/unit/test_input_handler.py b/tests/unit/test_input_handler.py index 70e33ba2..ec52b678 100644 --- a/tests/unit/test_input_handler.py +++ b/tests/unit/test_input_handler.py @@ -29,6 +29,7 @@ ALLOWED_GIT_HOSTS, InputHandler, _open_regular_file_from_windows_handle, + _open_regular_file_no_follow, ) @@ -233,6 +234,24 @@ def test_resolve_file_open_failure_does_not_create_temp_dir(tmp_path: Path) -> N handler.cleanup() +@pytest.mark.skipif(not hasattr(os, "O_PATH"), reason="requires O_PATH (Linux)") +@pytest.mark.skipif( + hasattr(os, "geteuid") and os.geteuid() == 0, reason="root bypasses directory permissions" +) +def test_secure_open_traverses_search_only_ancestors(tmp_path: Path) -> None: + """Traversal needs search access on ancestors, not read access.""" + parent = tmp_path / "search_only" + parent.mkdir() + source = parent / "SKILL.md" + source.write_text("# Skill", encoding="utf-8") + os.chmod(parent, 0o111) + try: + with _open_regular_file_no_follow(source) as opened: + assert opened.read() == b"# Skill" + finally: + os.chmod(parent, 0o755) + + def test_resolve_file_rejects_platform_without_safe_open_support( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: