Skip to content

Commit 1c4c858

Browse files
pwx603claude
andcommitted
fix: resolver 不再因 symlink 环抛异常,退化输入不再误报 ok
review 发现两处缺陷(均源自计划中的参考实现): - Critical:Path.resolve() 对循环 symlink 抛 RuntimeError(非 OSError/ ValueError),未被捕获,违反「resolve() 永不抛异常」的硬约束。 - Important:"///" / "." / "./" 归一化后指向 repo root,被误报为 ok/dir;原 junk 测试仅断言 status 取值合法,无法捕获。 补充回归测试:tmp_path 下构造 symlink 环、退化输入断言 missing。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9c49215 commit 1c4c858

2 files changed

Lines changed: 69 additions & 3 deletions

File tree

mcp/ccarch/ccarch/resolver.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,25 @@ def resolve(repo_root: Path, cited: str) -> Resolved:
3737

3838
# Never let a cited path escape the repo. A doc citing ../../etc/passwd
3939
# resolves to missing rather than probing outside the tree.
40+
repo_resolved = repo_root.resolve()
4041
try:
4142
target = (repo_root / q).resolve()
42-
target.relative_to(repo_root.resolve())
43-
except (ValueError, OSError):
43+
target.relative_to(repo_resolved)
44+
except (ValueError, OSError, RuntimeError):
45+
# Path.resolve() raises RuntimeError("Symlink loop from ...") for a
46+
# circular symlink instead of OSError/ValueError like every other
47+
# unresolvable-path case. It looks redundant to catch it here, but
48+
# it isn't — leave it, or a symlink loop in a doc-cited path will
49+
# crash resolve() instead of reporting "missing".
50+
return Resolved(path=raw, status="missing", kind="dir" if is_dir_cite else "file")
51+
52+
# Degenerate citations like "", "///", ".", "./" all normalize to the
53+
# repo root itself once joined and resolved (repo_root / "" is
54+
# repo_root; "." and "./" resolve to the same directory). None of these
55+
# actually names a path the doc could plausibly be citing, so treat a
56+
# cite that resolves to exactly the repo root as missing rather than an
57+
# "ok" hit on the whole repository.
58+
if target == repo_resolved:
4459
return Resolved(path=raw, status="missing", kind="dir" if is_dir_cite else "file")
4560

4661
if is_dir_cite:

mcp/ccarch/tests/test_resolver.py

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from pathlib import Path
2+
23
from ccarch.resolver import resolve
34

45
REPO = Path(__file__).resolve().parents[3]
@@ -26,5 +27,55 @@ def test_genuinely_missing_is_missing():
2627
assert r.status == "missing"
2728

2829
def test_never_raises_on_junk():
30+
# Every entry below is parser garbage or a fabricated/escaping path —
31+
# none of them names a real, in-repo file or dir. resolve() must not
32+
# raise, and must not mislabel any of them as "ok"/"moved".
2933
for junk in ["", " ", "///", "src/foo.ts", "../../etc/passwd"]:
30-
assert resolve(REPO, junk).status in {"ok", "moved", "missing"}
34+
assert resolve(REPO, junk).status == "missing"
35+
36+
37+
def test_degenerate_slash_and_dot_input_is_missing():
38+
# q = raw.rstrip("/") can collapse to "", and repo_root / "" is just
39+
# repo_root — which must never be reported as a legitimate directory hit.
40+
for junk in ["", " ", "///", ".", "./"]:
41+
r = resolve(REPO, junk)
42+
assert r.status == "missing", f"{junk!r} -> {r.status}"
43+
44+
45+
def test_symlink_escaping_repo_is_missing(tmp_path):
46+
# Reviewed/confirmed behavior: a symlink inside the repo pointing outside
47+
# it must still resolve to missing. Guard against regressing this while
48+
# fixing the symlink-loop RuntimeError below. Built under tmp_path so we
49+
# never write symlinks into the real repo tree.
50+
outside = tmp_path / "outside"
51+
outside.mkdir()
52+
(outside / "passwd").write_text("secret")
53+
54+
repo = tmp_path / "repo"
55+
(repo / "src").mkdir(parents=True)
56+
(repo / "src" / "escaped").symlink_to(outside, target_is_directory=True)
57+
58+
r = resolve(repo, "src/escaped/passwd")
59+
assert r.status == "missing"
60+
61+
62+
def test_symlink_loop_does_not_raise(tmp_path):
63+
# Path.resolve() raises RuntimeError("Symlink loop from ...") on a
64+
# circular symlink — not OSError/ValueError. resolve() must catch it
65+
# and report missing, never propagate.
66+
(tmp_path / "docs").mkdir()
67+
src = tmp_path / "src"
68+
src.mkdir()
69+
(src / "loopa").symlink_to(src / "loopb")
70+
(src / "loopb").symlink_to(src / "loopa")
71+
72+
r = resolve(tmp_path, "src/loopa/file.ts")
73+
assert r.status == "missing"
74+
75+
76+
def test_broken_symlink_does_not_raise(tmp_path):
77+
(tmp_path / "src").mkdir()
78+
(tmp_path / "src" / "ghost").symlink_to(tmp_path / "src" / "nonexistent")
79+
80+
r = resolve(tmp_path, "src/ghost")
81+
assert r.status == "missing"

0 commit comments

Comments
 (0)