Skip to content

Commit 7af372f

Browse files
fix(mcp): a symlinked workspace root contains its own files
Path membership is lexical, so a root that still carries a symlink (macOS /tmp or /var, a symlinked home) matched none of the resolved paths checked against it -- which silently voided the main checkout added to the allow-list. Resolve both sides in tool_smart_edit, rich_edit._resolve and _resolve_explicit_edit_root; the /tmp twin entry is redundant now. Co-Authored-By: lemoncrow <302591943+lemoncrow-agent[bot]@users.noreply.github.com> LemonCrow-Session: s1
1 parent 55d137b commit 7af372f

3 files changed

Lines changed: 58 additions & 10 deletions

File tree

src/lemoncrow/gateway/adapters/mcp_server.py

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6929,7 +6929,9 @@ def _resolve_explicit_edit_root(raw_root: str, *, workspace_root: Path, extra_ro
69296929
return candidate
69306930
if _linked_worktree_root(workspace_root, candidate) is not None:
69316931
return candidate
6932-
if any(candidate == r or candidate.is_relative_to(r) for r in extra_roots):
6932+
# Resolved on both sides: an additional directory reached through a symlink
6933+
# (or the /tmp literal on macOS) contains nothing lexically.
6934+
if any(candidate == r or candidate.is_relative_to(r) for r in (extra.resolve() for extra in extra_roots)):
69336935
return candidate
69346936
return None
69356937

@@ -7851,10 +7853,9 @@ def tool_smart_edit(
78517853
# Confine writes to the workspace root plus any additional directories from
78527854
# Claude Code's additionalDirectories setting or LEMONCROW_ADDITIONAL_DIRS env.
78537855
# Read tools accept any absolute path; writes need explicit opt-in.
7854-
# Path("/tmp").resolve() as well as "/tmp": on macOS /tmp is a symlink to
7855-
# /private/tmp, and the candidates below are resolved, so the bare literal
7856-
# never matched and the /tmp allowance was dead on that platform.
7857-
_extra_roots = [*_claude_additional_dirs(repo_root), Path("/tmp"), Path("/tmp").resolve()]
7856+
# "/tmp" needs no twin "/private/tmp" entry: _allowed_edit_roots resolves
7857+
# every root before comparing, so one literal covers macOS's symlink.
7858+
_extra_roots = [*_claude_additional_dirs(repo_root), Path("/tmp")]
78587859
if _session_worktree is not None:
78597860
_extra_roots.append(_session_worktree)
78607861

@@ -7880,7 +7881,18 @@ def tool_smart_edit(
78807881
}
78817882
_extra_roots.append(_explicit_root)
78827883
_edit_root = _explicit_root or _session_worktree or repo_root
7883-
_allowed_edit_roots = [repo_root, _edit_root, *_extra_roots]
7884+
# Resolved against resolved. Touched paths arrive through
7885+
# _resolve_snapshot_path's .resolve(), while _workspace_root() hands back
7886+
# whatever the env or CLI gave it -- a macOS /tmp or /var path, a home
7887+
# reached through a symlink -- and is_relative_to is purely lexical, so an
7888+
# unresolved root lexically contains none of its own files. Compare the
7889+
# resolved forms; the escape error still prints the caller's own path.
7890+
_allowed_edit_roots = [_candidate.resolve() for _candidate in (repo_root, _edit_root, *_extra_roots)]
7891+
# Every later membership test below takes a RESOLVED path, so it needs the
7892+
# resolved root for the same reason -- under a symlinked workspace an
7893+
# unresolved one silently drops all hook diagnostics and every path the
7894+
# contract review would have read.
7895+
_repo_root_resolved = repo_root.resolve()
78847896

78857897
# A relative path naming an existing file in BOTH the inferred worktree and
78867898
# the workspace root has no right answer: the worktree came from another
@@ -8277,7 +8289,7 @@ def _diag_in_repo_root(d: dict[str, Any], root: Path) -> bool:
82778289
result["diagnostics"] = [
82788290
d
82798291
for d in result["diagnostics"]
8280-
if d.get("severity") in ("error", "warning") and _diag_in_repo_root(d, repo_root)
8292+
if d.get("severity") in ("error", "warning") and _diag_in_repo_root(d, _repo_root_resolved)
82818293
]
82828294
if not result["diagnostics"]:
82838295
result.pop("diagnostics")
@@ -8294,7 +8306,7 @@ def _fmt_diag(d: dict[str, Any], root: Path) -> str:
82948306
msg = d.get("message", "")
82958307
return f"{loc} {code}: {msg}" if code else f"{loc}: {msg}"
82968308

8297-
_diag_lines = [_fmt_diag(d, repo_root) for d in result.pop("diagnostics")]
8309+
_diag_lines = [_fmt_diag(d, _repo_root_resolved) for d in result.pop("diagnostics")]
82988310
# Cap: a touched file with many pre-existing findings must not dump
82998311
# an unbounded lint report into the edit result.
83008312
if len(_diag_lines) > _EDIT_DIAG_CAP:
@@ -8340,7 +8352,9 @@ def _fmt_diag(d: dict[str, Any], root: Path) -> str:
83408352
result,
83418353
edits,
83428354
repo_root=repo_root,
8343-
touched_paths=[str(p.relative_to(repo_root)) for p in paths.values() if p.is_relative_to(repo_root)],
8355+
touched_paths=[
8356+
str(p.relative_to(_repo_root_resolved)) for p in paths.values() if p.is_relative_to(_repo_root_resolved)
8357+
],
83448358
)
83458359
_phase_contract_ms = int((time.monotonic() - _contract_start) * 1000)
83468360
# Incremental: refresh the shared index for the touched files now, so a

src/lemoncrow/pro/capabilities/tool_supervision/rich_edit.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,11 @@ def _resolve(root: Path, raw_path: str, allowed_roots: list[Path] | None = None)
137137
path = Path(spec.path)
138138
resolved = path if path.is_absolute() else root / path
139139
resolved = resolved.resolve()
140-
roots = [root, *(allowed_roots or [])]
140+
# `resolved` is resolved, so the roots must be too: is_relative_to is
141+
# lexical, and a root carrying a symlink (macOS /tmp, /var, a symlinked
142+
# home) lexically contains none of its own files. The message below still
143+
# names `root` as the caller passed it.
144+
roots = [Path(r).resolve() for r in (root, *(allowed_roots or []))]
141145
if not any(resolved == r or resolved.is_relative_to(r) for r in roots):
142146
raise ValueError(
143147
f"path escape denied: {raw_path} is outside the workspace root {root} — "

tests/gateway/test_edit_mcp_handler.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1563,6 +1563,36 @@ def test_absolute_main_checkout_path_survives_a_worktree_inference(
15631563
assert target.read_text(encoding="utf-8") == "EDITED\n"
15641564

15651565

1566+
def test_absolute_edit_under_a_symlinked_workspace_root_is_not_an_escape(
1567+
workspace: Path, monkeypatch: pytest.MonkeyPatch
1568+
) -> None:
1569+
"""A workspace reached through a symlink still contains its own files.
1570+
1571+
Touched paths come back resolved, while the workspace root arrives however
1572+
the env handed it over -- and ``is_relative_to`` is lexical, so an
1573+
unresolved root lexically contains none of its own files. On macOS every
1574+
workspace under /tmp or /var reaches the handler this way.
1575+
"""
1576+
real = workspace / "real"
1577+
real.mkdir()
1578+
link = workspace / "link"
1579+
link.symlink_to(real, target_is_directory=True)
1580+
monkeypatch.setenv("CLAUDE_WORKSPACE_ROOT", str(link))
1581+
wt = _repo_with_worktree(real)
1582+
monkeypatch.setattr(mcp_server, "_last_session_cwd", str(wt))
1583+
(link / "main_only.txt").write_text("MAIN\n", encoding="utf-8")
1584+
1585+
payload = _edit(
1586+
{
1587+
"post_edit_hooks": False,
1588+
"edits": [{"file_path": str(link / "main_only.txt"), "old_string": "MAIN", "new_string": "EDITED"}],
1589+
}
1590+
)
1591+
1592+
assert "failed" not in payload, payload
1593+
assert (real / "main_only.txt").read_text(encoding="utf-8") == "EDITED\n"
1594+
1595+
15661596
def test_bash_cwd_is_what_teaches_edit_where_the_session_is(monkeypatch: pytest.MonkeyPatch) -> None:
15671597
"""Only a bash call's cwd is recorded -- it is the sole session-cwd signal."""
15681598
monkeypatch.setattr(mcp_server, "_last_session_cwd", None)

0 commit comments

Comments
 (0)