fix(server): guard project_root writes against mock/relative coercion (TAP-4573)

packages/tapps-core/src/tapps_core/agent_identity.py

@@ -35,6 +35,25 @@
 _SLUG_INVALID_RE = re.compile(r"[^a-zA-Z0-9_-]+")
 
 
+def is_real_writable_root(project_root: object) -> bool:
+    """Return True only when *project_root* is a real, absolute filesystem path.
+
+    TAP-4573 guard. Production callers always resolve ``project_root`` to an
+    absolute path (explicit arg, ``TAPPS_MCP_PROJECT_ROOT`` env, or ``cwd``).
+    A bare ``MagicMock()`` coerces via ``os.fspath`` to the *relative* string
+    ``MagicMock/mock.project_root/<id>`` (verified empirically), so ~70 test
+    call sites that pass unspec'd mocks were causing real ``mkdir`` trees under
+    the pytest CWD (the repo root). Rejecting non-absolute / non-coercible
+    roots blocks that leak at the single production write path without touching
+    the tests, and is a no-op for every real deployment (roots are absolute).
+    """
+    try:
+        raw = os.fspath(project_root)  # type: ignore[arg-type]
+    except (TypeError, ValueError):
+        return False
+    return Path(raw).is_absolute()
+
+
 def _slugify(value: str) -> str:
     """Reduce a string to a safe agent-id prefix (alnum/dash/underscore)."""
     cleaned = _SLUG_INVALID_RE.sub("-", value).strip("-_")
@@ -86,7 +105,16 @@ def get_stable_agent_id(settings: TappsMCPSettings) -> str:
     if override:
         return override
 
-    project_root = Path(getattr(settings, "project_root", Path.cwd()))
+    raw_root = getattr(settings, "project_root", Path.cwd())
+
+    # TAP-4573: never mkdir/write under a non-real (mock- or relative-coerced)
+    # project_root. A bare MagicMock() coerces to a relative "MagicMock/..."
+    # path, so writing it would create a real tree in the pytest CWD. Skip the
+    # persistence attempt but still return a valid in-memory agent id.
+    if not is_real_writable_root(raw_root):
+        return f"{_project_slug(settings)}-{uuid.uuid4().hex[:_UUID_SHORT_LEN]}"
+
+    project_root = Path(raw_root)
     id_path = project_root / _AGENT_ID_RELATIVE_PATH
 
     uuid_hex = _read_uuid(id_path)
@@ -113,4 +141,4 @@ def get_stable_agent_id(settings: TappsMCPSettings) -> str:
     return f"{_project_slug(settings)}-{short}"
 
 
-__all__ = ["get_stable_agent_id"]
+__all__ = ["get_stable_agent_id", "is_real_writable_root"]

packages/tapps-core/tests/unit/test_agent_identity.py

@@ -5,8 +5,12 @@
 import re
 from pathlib import Path
 from typing import TYPE_CHECKING
+from unittest.mock import MagicMock
 
-from tapps_core.agent_identity import get_stable_agent_id
+from tapps_core.agent_identity import (
+    get_stable_agent_id,
+    is_real_writable_root,
+)
 from tapps_core.config.settings import TappsMCPSettings
 
 if TYPE_CHECKING:
@@ -115,3 +119,43 @@ def test_agent_id_slugifies_unsafe_project_id(
 
     assert agent_id.startswith("my-project-v2-")
     assert _AGENT_ID_RE.match(agent_id)
+
+
+# --------------------------------------------------------------------------- #
+# TAP-4573: mock/relative-root write guard
+# --------------------------------------------------------------------------- #
+
+
+def test_is_real_writable_root_rejects_mock_and_relative() -> None:
+    """The guard accepts only absolute filesystem paths."""
+    assert is_real_writable_root(Path("/abs/real/path")) is True
+    assert is_real_writable_root("/abs/real/path") is True
+    # A bare MagicMock coerces (via os.fspath) to a *relative* "MagicMock/..."
+    # path — the exact leak vector from TAP-4573.
+    assert is_real_writable_root(MagicMock().project_root) is False
+    assert is_real_writable_root(Path("relative/dir")) is False
+    assert is_real_writable_root(object()) is False
+
+
+def test_mock_settings_creates_no_files(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """A bare MagicMock() settings must not mkdir/write under the CWD.
+
+    Regression guard for TAP-4573: ~70 call sites passed unspec'd mocks whose
+    ``project_root`` coerced to a relative ``MagicMock/...`` path, leaking real
+    directory trees into the repo root on every full-suite run.
+    """
+    monkeypatch.delenv("CLAUDE_AGENT_ID", raising=False)
+    monkeypatch.chdir(tmp_path)
+
+    settings = MagicMock()
+    settings.memory.project_id = ""
+
+    agent_id = get_stable_agent_id(settings)
+
+    # Still returns a well-formed id (in-memory UUID, no persistence).
+    assert _AGENT_ID_RE.match(agent_id), f"unexpected format: {agent_id!r}"
+    # And critically: no MagicMock/ tree was created under the CWD.
+    assert not (tmp_path / "MagicMock").exists()
+    assert list(tmp_path.iterdir()) == []

packages/tapps-mcp/src/tapps_mcp/server_helpers.py

@@ -654,6 +654,13 @@ def write_session_start_marker(project_root: Path | str) -> None:
     user prompt yields a reminder.
     """
     try:
+        # TAP-4573: refuse to mkdir/write under a non-real project_root. A bare
+        # MagicMock() coerces to a relative "MagicMock/..." path, which would
+        # otherwise create a real tree in the pytest CWD (the repo root).
+        from tapps_core.agent_identity import is_real_writable_root
+
+        if not is_real_writable_root(project_root):
+            return
         root = project_root if isinstance(project_root, Path) else Path(project_root)
         sidecar_dir = root / ".tapps-mcp"
         sidecar_dir.mkdir(parents=True, exist_ok=True)

packages/tapps-mcp/tests/unit/test_user_prompt_submit_hook.py

@@ -59,6 +59,17 @@ def test_session_start_marker_swallows_errors(self) -> None:
         # Writer must not raise.
         write_session_start_marker("/dev/null/cannot-mkdir-here")
 
+    def test_session_start_marker_ignores_mock_root(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        # TAP-4573: a bare MagicMock().project_root coerces to a relative
+        # "MagicMock/..." path. The writer must refuse it rather than create a
+        # real tree under the pytest CWD.
+        monkeypatch.chdir(tmp_path)
+        write_session_start_marker(MagicMock().project_root)
+        assert not (tmp_path / "MagicMock").exists()
+        assert list(tmp_path.iterdir()) == []
+
     def test_checklist_state_marker_emits_brain_event(self, tmp_path: Path) -> None:
         mock_bridge = MagicMock()
         mock_bridge.record_kg_event = AsyncMock(return_value={"recorded": True})