Skip to content

Commit dcb5a00

Browse files
pwx603claude
andcommitted
fix: guard EACCES on filesystem probes in ccarch resolver
Path.is_file()/is_dir() only swallow ENOENT/ENOTDIR/EBADF/ELOOP internally (pathlib._IGNORED_ERRNOS); EACCES re-raises as PermissionError. A permission-denied path/ancestor dir (locked-down checkout dir, different-UID Docker mount) crashed resolve() instead of reporting "missing", breaching the never-raise contract for the third time. Add _safe_check() helper wrapping is_file()/is_dir() in try/except OSError, and route all four probes in resolve() through it. Add regression test using chmod 000 under tmp_path, skipped under root (chmod 000 doesn't block root) and restoring permissions in finally so tmp_path teardown doesn't choke on the unreadable dir. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1c4c858 commit dcb5a00

2 files changed

Lines changed: 43 additions & 4 deletions

File tree

mcp/ccarch/ccarch/resolver.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,21 @@ class Resolved:
1818
kind: str # "file" | "dir"
1919

2020

21+
def _safe_check(p: Path, kind: str) -> bool:
22+
# Path.is_file()/is_dir() only swallow a fixed errno set internally
23+
# (pathlib._IGNORED_ERRNOS = ENOENT, ENOTDIR, EBADF, ELOOP) and re-raise
24+
# anything else as OSError. EACCES — a permission-denied path or an
25+
# ancestor directory we can't stat through, e.g. a locked-down directory
26+
# in the checkout or a different-UID Docker mount — is not in that set,
27+
# so an unguarded call can crash resolve() on a perfectly realistic
28+
# filesystem state. Treat any such probe failure as "not found" rather
29+
# than letting it propagate: resolve() must never raise.
30+
try:
31+
return p.is_file() if kind == "file" else p.is_dir()
32+
except OSError:
33+
return False
34+
35+
2136
def _swap_ext(p: str) -> str | None:
2237
if p.endswith(".ts"):
2338
return p[:-3] + ".tsx"
@@ -61,17 +76,17 @@ def resolve(repo_root: Path, cited: str) -> Resolved:
6176
if is_dir_cite:
6277
return Resolved(
6378
path=raw,
64-
status="ok" if target.is_dir() else "missing",
79+
status="ok" if _safe_check(target, "dir") else "missing",
6580
kind="dir",
6681
)
6782

68-
if target.is_file():
83+
if _safe_check(target, "file"):
6984
return Resolved(path=q, status="ok", kind="file")
70-
if target.is_dir():
85+
if _safe_check(target, "dir"):
7186
return Resolved(path=q, status="ok", kind="dir")
7287

7388
alt = _swap_ext(q)
74-
if alt and (repo_root / alt).is_file():
89+
if alt and _safe_check(repo_root / alt, "file"):
7590
return Resolved(path=alt, status="moved", kind="file")
7691

7792
return Resolved(path=q, status="missing", kind="file")

mcp/ccarch/tests/test_resolver.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
import os
12
from pathlib import Path
23

4+
import pytest
5+
36
from ccarch.resolver import resolve
47

58
REPO = Path(__file__).resolve().parents[3]
@@ -79,3 +82,24 @@ def test_broken_symlink_does_not_raise(tmp_path):
7982

8083
r = resolve(tmp_path, "src/ghost")
8184
assert r.status == "missing"
85+
86+
87+
@pytest.mark.skipif(
88+
os.geteuid() == 0,
89+
reason="chmod 000 does not block root, so this would pass vacuously",
90+
)
91+
def test_permission_denied_dir_does_not_raise(tmp_path):
92+
# Path.is_file()/is_dir() only swallow ENOENT/ENOTDIR/EBADF/ELOOP
93+
# (pathlib._IGNORED_ERRNOS); EACCES is not in that set, so a
94+
# permission-denied ancestor directory makes them re-raise
95+
# PermissionError. resolve() must still report "missing", not raise.
96+
locked = tmp_path / "src" / "locked"
97+
locked.mkdir(parents=True)
98+
(locked / "secret.ts").write_text("secret")
99+
os.chmod(locked, 0o000)
100+
try:
101+
r = resolve(tmp_path, "src/locked/secret.ts")
102+
assert r.status == "missing"
103+
finally:
104+
# Restore permissions so tmp_path teardown can clean up the dir.
105+
os.chmod(locked, 0o755)

0 commit comments

Comments
 (0)