diff --git a/mcp_server/handlers/open_visualization.py b/mcp_server/handlers/open_visualization.py index 91efb580..bf071d22 100644 --- a/mcp_server/handlers/open_visualization.py +++ b/mcp_server/handlers/open_visualization.py @@ -60,6 +60,34 @@ def _find_dev_source() -> Path | None: duplicated here so this handler stays usable even when it's loaded from an older plugin-cache snapshot whose launcher lacks the auto-detect extension. + + Security gating (GHSA-gvpp-v77h-5w8g, EQSTLab 2026-05-27): the + return value of this function is consumed by ``handler()`` to + locate a ``visualize_bootstrap.py`` that is then ``subprocess.run`` + against the local Python interpreter. Any directory we return is + therefore a code-execution surface, so candidate sources must NOT + be attacker-controllable. + + Previous implementation accepted ``CLAUDE_PROJECT_DIR`` (set + automatically by Claude Code to whatever project the user opens) + as a candidate, validated by a two-marker-file check + (``mcp_server/`` directory + ``ui/unified-viz.html``) that any + attacker can trivially replicate. That allowed local arbitrary + code execution by enticing the user to open an attacker-crafted + project and run ``/cortex-visualize``. + + Hardening: + * ``CLAUDE_PROJECT_DIR`` is no longer consulted. + * ``CORTEX_DEV_ROOT`` is consulted only when the user has also + set ``CORTEX_DEV_SOURCE_SYNC=1`` — an explicit opt-in flag + that signals "I deliberately want my CORTEX_DEV_ROOT to be + used as a code-execution dev source." Without the flag, + ``CORTEX_DEV_ROOT`` (which an attacker could in principle + plant in a shell rc file) is ignored. + * The conventional ``~/Documents/Developments/Cortex`` fallback + remains — that path is controlled by the user's own filesystem + and an attacker who can already write under ``$HOME`` has + higher-privilege code execution by other means. """ def _is_cortex_root(p: Path) -> bool: @@ -70,10 +98,12 @@ def _is_cortex_root(p: Path) -> bool: ) candidates: list[Path] = [] - for env in ("CORTEX_DEV_ROOT", "CLAUDE_PROJECT_DIR"): - v = os.environ.get(env) + # Explicit dev-source opt-in (see security gating in docstring). + if os.environ.get("CORTEX_DEV_SOURCE_SYNC") == "1": + v = os.environ.get("CORTEX_DEV_ROOT") if v: candidates.append(Path(v)) + # Conventional home-directory checkout — user-controlled, safe. candidates.append(Path.home() / "Documents" / "Developments" / "Cortex") for c in candidates: if _is_cortex_root(c): diff --git a/mcp_server/server/http_launcher.py b/mcp_server/server/http_launcher.py index 8fb57d60..46df384a 100644 --- a/mcp_server/server/http_launcher.py +++ b/mcp_server/server/http_launcher.py @@ -51,16 +51,31 @@ def _kill_port(port: int) -> None: def _detect_dev_source() -> Path | None: """Return a dev-checkout source root if one is visible. - Detection order: - 1. ``CORTEX_DEV_ROOT`` env var — explicit override. - 2. ``CLAUDE_PROJECT_DIR`` env var — Claude Code sets this when - the user is working inside a project directory. - 3. The file the launcher module was loaded from, if it's inside - a Cortex source tree (auto-detect for dev mode). - 4. The conventional checkout location - ``$HOME/Documents/Developments/Cortex`` — falls back here so - the MCP itself syncs on every ``cortex-visualize`` call with - no env-var configuration. + Detection order (after the GHSA-gvpp-v77h-5w8g hardening): + 1. ``CORTEX_DEV_ROOT`` env var, **only when** the user has also + set ``CORTEX_DEV_SOURCE_SYNC=1`` as an explicit opt-in. The + flag signals "I deliberately want my CORTEX_DEV_ROOT to be + used as a code-execution dev source"; without it the env + var is ignored. + 2. The file the launcher module was loaded from, if it's inside + a Cortex source tree (auto-detect for ``pip install -e .`` + / ``uv run`` dev mode). This is filesystem-position-based: + the attacker would have to place the launcher module itself + inside their malicious project to influence it, which + requires write access to the user's site-packages and is + therefore higher-privileged than the exploit it would yield. + 3. The conventional checkout location + ``$HOME/Documents/Developments/Cortex`` — controlled by the + user's own filesystem. + + ``CLAUDE_PROJECT_DIR`` (set automatically by Claude Code to + whatever project the user has open) is **NOT** consulted: per + EQSTLab's 2026-05-27 advisory, that path is attacker-controllable + via social-engineering ("open this repo to reproduce the bug") + and combined with the two-marker ``_is_cortex_root`` check it + constituted a local arbitrary-code-execution surface (the + returned dev source is passed to ``rsync`` and then the + visualization server respawns from the synced copy). A directory qualifies only if it contains both ``mcp_server/`` and ``ui/unified-viz.html``. When a dev source is returned @@ -77,17 +92,19 @@ def _is_cortex_root(p: Path) -> bool: ) candidates: list[Path] = [] - for env in ("CORTEX_DEV_ROOT", "CLAUDE_PROJECT_DIR"): - v = os.environ.get(env) + # Explicit dev-source opt-in (see security gating in docstring). + if os.environ.get("CORTEX_DEV_SOURCE_SYNC") == "1": + v = os.environ.get("CORTEX_DEV_ROOT") if v: candidates.append(Path(v)) # Walk up from this module to see if we're loaded out of a source - # checkout (for ``uv run`` / ``pip install -e`` dev mode). + # checkout (for ``uv run`` / ``pip install -e`` dev mode). Position + # of the module on disk; not attacker-controllable through env. here = Path(__file__).resolve() for ancestor in list(here.parents)[:6]: candidates.append(ancestor) # Conventional location — the MCP plugin auto-syncs from here even - # when no env var is set. + # when no env var is set. User-controlled filesystem path. candidates.append(Path.home() / "Documents" / "Developments" / "Cortex") for c in candidates: diff --git a/mcp_server/server/visualize_bootstrap.py b/mcp_server/server/visualize_bootstrap.py index 335892a1..2a8577a9 100644 --- a/mcp_server/server/visualize_bootstrap.py +++ b/mcp_server/server/visualize_bootstrap.py @@ -43,8 +43,19 @@ def _is_cortex_root(p: Path) -> bool: def _find_dev_source() -> Path | None: - for env in ("CORTEX_DEV_ROOT", "CLAUDE_PROJECT_DIR"): - v = os.environ.get(env) + """Locate the dev source. See GHSA-gvpp-v77h-5w8g gating in + ``mcp_server/handlers/open_visualization._find_dev_source`` — the + bootstrap script inherits the parent process environment, so any + untrusted env var consulted here would re-open the same hole the + handler closes. + + ``CLAUDE_PROJECT_DIR`` is therefore NOT consulted. ``CORTEX_DEV_ROOT`` + requires the explicit ``CORTEX_DEV_SOURCE_SYNC=1`` opt-in flag + (exact value ``"1"``). The ``~/Documents/Developments/Cortex`` + fallback is preserved (user-controlled filesystem). + """ + if os.environ.get("CORTEX_DEV_SOURCE_SYNC") == "1": + v = os.environ.get("CORTEX_DEV_ROOT") if v and _is_cortex_root(Path(v)): return Path(v) default = Path.home() / "Documents" / "Developments" / "Cortex" diff --git a/pyproject.toml b/pyproject.toml index faae9a80..212d01a4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "neuro-cortex-memory" -version = "3.17.0" +version = "3.17.1" description = "Scientifically-grounded memory system based on computational neuroscience research" readme = "README.md" license = "MIT" diff --git a/tests_py/handlers/test_open_visualization.py b/tests_py/handlers/test_open_visualization.py index 4040da9b..35729986 100644 --- a/tests_py/handlers/test_open_visualization.py +++ b/tests_py/handlers/test_open_visualization.py @@ -1,10 +1,12 @@ """Tests for mcp_server.handlers.open_visualization — unified 3D graph launcher.""" import asyncio +import tempfile +from pathlib import Path from unittest.mock import patch, MagicMock from mcp_server.handlers import open_visualization -from mcp_server.handlers.open_visualization import handler +from mcp_server.handlers.open_visualization import _find_dev_source, handler class TestOpenVisualizationSchema: @@ -115,3 +117,113 @@ def test_opens_browser_at_tilemap_url_unconditionally(self): result = asyncio.run(handler({})) mock_open.assert_called_once_with("http://localhost:5555/?viz=tilemap") assert "tilemap" in result["message"] + + +class TestDevSourceSecurityHardening: + """Falsification tests for GHSA-gvpp-v77h-5w8g — `_find_dev_source` + must not be persuadable by attacker-controllable env vars. + + Each test would fail if a regression re-introduced + ``CLAUDE_PROJECT_DIR`` as a candidate, or removed the explicit + ``CORTEX_DEV_SOURCE_SYNC=1`` opt-in for ``CORTEX_DEV_ROOT``. The + threat model is: an attacker tricks the user into opening a + malicious project in Claude Code (which sets + ``CLAUDE_PROJECT_DIR``); the malicious project contains the two + marker files (``mcp_server/`` directory + ``ui/unified-viz.html``) + that ``_is_cortex_root`` checks, plus a + ``mcp_server/server/visualize_bootstrap.py`` containing arbitrary + Python. When the user runs ``/cortex-visualize`` the handler used + to ``subprocess.run`` that file, giving the attacker local ACE. + """ + + @staticmethod + def _plant_marker_files(root: Path) -> None: + (root / "mcp_server" / "server").mkdir(parents=True, exist_ok=True) + (root / "ui").mkdir(parents=True, exist_ok=True) + (root / "ui" / "unified-viz.html").write_text( + "attacker", encoding="utf-8" + ) + (root / "mcp_server" / "server" / "visualize_bootstrap.py").write_text( + "raise RuntimeError('attacker-controlled bootstrap')\n", + encoding="utf-8", + ) + + def test_claude_project_dir_is_ignored(self, monkeypatch): + # Falsifies: CLAUDE_PROJECT_DIR can drive _find_dev_source. + with tempfile.TemporaryDirectory(prefix="cortex-malicious-") as td: + attacker_root = Path(td) + self._plant_marker_files(attacker_root) + # Make sure no other env var or home-fallback satisfies + # the search — otherwise the test would be vacuous. + monkeypatch.delenv("CORTEX_DEV_SOURCE_SYNC", raising=False) + monkeypatch.delenv("CORTEX_DEV_ROOT", raising=False) + monkeypatch.setenv("CLAUDE_PROJECT_DIR", str(attacker_root)) + with patch( + "mcp_server.handlers.open_visualization.Path.home", + return_value=attacker_root.parent, + ): + # parent dir contains the malicious dir but lacks + # ``Documents/Developments/Cortex`` so home-fallback + # cannot accidentally satisfy. + result = _find_dev_source() + assert result is None, ( + f"CLAUDE_PROJECT_DIR should be ignored; got {result!r} — " + "this would re-introduce GHSA-gvpp-v77h-5w8g." + ) + + def test_cortex_dev_root_ignored_without_opt_in(self, monkeypatch): + # Falsifies: CORTEX_DEV_ROOT is honoured without the + # CORTEX_DEV_SOURCE_SYNC=1 flag. + with tempfile.TemporaryDirectory(prefix="cortex-unopted-") as td: + attacker_root = Path(td) + self._plant_marker_files(attacker_root) + monkeypatch.delenv("CORTEX_DEV_SOURCE_SYNC", raising=False) + monkeypatch.setenv("CORTEX_DEV_ROOT", str(attacker_root)) + monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) + with patch( + "mcp_server.handlers.open_visualization.Path.home", + return_value=attacker_root.parent, + ): + result = _find_dev_source() + assert result is None, ( + f"CORTEX_DEV_ROOT honoured without the opt-in flag; got {result!r}." + ) + + def test_cortex_dev_root_honoured_when_explicitly_opted_in(self, monkeypatch): + # Falsifies: opt-in flag is broken (legitimate dev workflow + # would also break). This test exists so we don't over-lock + # the door and lose the intended developer affordance. + with tempfile.TemporaryDirectory(prefix="cortex-dev-real-") as td: + dev_root = Path(td) + self._plant_marker_files(dev_root) + monkeypatch.setenv("CORTEX_DEV_SOURCE_SYNC", "1") + monkeypatch.setenv("CORTEX_DEV_ROOT", str(dev_root)) + monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) + with patch( + "mcp_server.handlers.open_visualization.Path.home", + return_value=dev_root.parent, + ): + result = _find_dev_source() + assert result == dev_root, ( + f"Opt-in path broken: expected {dev_root!r}, got {result!r}." + ) + + def test_opt_in_flag_value_not_1_is_rejected(self, monkeypatch): + # Falsifies: ANY non-empty CORTEX_DEV_SOURCE_SYNC value + # activates the gate (would let an accidental "true"/"yes" + # in a shell rc file pull in CORTEX_DEV_ROOT). + with tempfile.TemporaryDirectory(prefix="cortex-truthy-") as td: + root = Path(td) + self._plant_marker_files(root) + monkeypatch.setenv("CORTEX_DEV_SOURCE_SYNC", "true") + monkeypatch.setenv("CORTEX_DEV_ROOT", str(root)) + monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) + with patch( + "mcp_server.handlers.open_visualization.Path.home", + return_value=root.parent, + ): + result = _find_dev_source() + assert result is None, ( + "Gate must require the exact string '1' to avoid " + "ambiguous truthy values silently re-opening the hole." + ) diff --git a/tests_py/server/test_http_launcher_security.py b/tests_py/server/test_http_launcher_security.py new file mode 100644 index 00000000..f1c8f7e8 --- /dev/null +++ b/tests_py/server/test_http_launcher_security.py @@ -0,0 +1,172 @@ +"""Security regression tests for mcp_server.server.http_launcher and +mcp_server.server.visualize_bootstrap. + +Companion to ``tests_py/handlers/test_open_visualization.py``: the +launcher and the bootstrap script each expose a near-identical +``_find_dev_source`` / ``_detect_dev_source`` function that was also +part of the GHSA-gvpp-v77h-5w8g surface (their return values are +rsynced into the package path and the server is respawned from the +synced copy — equivalent code-execution semantics to the handler's +bootstrap call). + +The bootstrap script is the more subtle case: it's invoked by the +primary handler via ``subprocess.Popen``, inheriting the parent +process environment. So even though the handler's fix blocks the +obvious entry, the bootstrap MUST consult the same hardened rules — +otherwise any future code path that invokes ``visualize_bootstrap`` +(directly or via subprocess) re-opens the rsync overwrite hole. + +Each test would FAIL if a regression re-introduced +``CLAUDE_PROJECT_DIR`` as a candidate, or relaxed the explicit +``CORTEX_DEV_SOURCE_SYNC=1`` opt-in gate for ``CORTEX_DEV_ROOT``. +""" + +from __future__ import annotations + +import tempfile +from pathlib import Path +from unittest.mock import patch + +from mcp_server.server.http_launcher import _detect_dev_source +from mcp_server.server.visualize_bootstrap import ( + _find_dev_source as bootstrap_find_dev_source, +) + + +def _plant_marker_files(root: Path) -> None: + """Mirror of ``TestDevSourceSecurityHardening._plant_marker_files`` — + the same two markers ``_is_cortex_root`` checks, plus the bootstrap + script the original PoC exploited.""" + (root / "mcp_server" / "server").mkdir(parents=True, exist_ok=True) + (root / "ui").mkdir(parents=True, exist_ok=True) + (root / "ui" / "unified-viz.html").write_text( + "attacker", encoding="utf-8" + ) + (root / "mcp_server" / "server" / "visualize_bootstrap.py").write_text( + "raise RuntimeError('attacker-controlled bootstrap')\n", + encoding="utf-8", + ) + + +class TestDetectDevSourceSecurityHardening: + def test_claude_project_dir_is_ignored(self, monkeypatch): + # Falsifies: CLAUDE_PROJECT_DIR can drive _detect_dev_source — + # the secondary code-execution path called out in + # GHSA-gvpp-v77h-5w8g. + with tempfile.TemporaryDirectory(prefix="cortex-launcher-malicious-") as td: + attacker_root = Path(td) + _plant_marker_files(attacker_root) + monkeypatch.delenv("CORTEX_DEV_SOURCE_SYNC", raising=False) + monkeypatch.delenv("CORTEX_DEV_ROOT", raising=False) + monkeypatch.setenv("CLAUDE_PROJECT_DIR", str(attacker_root)) + with patch( + "mcp_server.server.http_launcher.Path.home", + return_value=attacker_root.parent, + ): + result = _detect_dev_source() + # Permissible non-None result: the launcher module's own + # ancestor walk may find a real Cortex checkout (this test + # is run inside one). Forbidden: returning the attacker + # root directly. + assert result != attacker_root, ( + f"CLAUDE_PROJECT_DIR should not be honoured; got {result!r} — " + "regression of GHSA-gvpp-v77h-5w8g." + ) + + def test_cortex_dev_root_ignored_without_opt_in(self, monkeypatch): + # Falsifies: CORTEX_DEV_ROOT is honoured without the + # CORTEX_DEV_SOURCE_SYNC=1 flag. + with tempfile.TemporaryDirectory(prefix="cortex-launcher-unopted-") as td: + attacker_root = Path(td) + _plant_marker_files(attacker_root) + monkeypatch.delenv("CORTEX_DEV_SOURCE_SYNC", raising=False) + monkeypatch.setenv("CORTEX_DEV_ROOT", str(attacker_root)) + monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) + with patch( + "mcp_server.server.http_launcher.Path.home", + return_value=attacker_root.parent, + ): + result = _detect_dev_source() + assert result != attacker_root, ( + f"CORTEX_DEV_ROOT honoured without the opt-in flag; got {result!r}." + ) + + def test_opt_in_flag_value_not_1_is_rejected(self, monkeypatch): + # Falsifies: ANY non-empty CORTEX_DEV_SOURCE_SYNC value + # activates the gate. + with tempfile.TemporaryDirectory(prefix="cortex-launcher-truthy-") as td: + root = Path(td) + _plant_marker_files(root) + monkeypatch.setenv("CORTEX_DEV_SOURCE_SYNC", "true") + monkeypatch.setenv("CORTEX_DEV_ROOT", str(root)) + monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) + with patch( + "mcp_server.server.http_launcher.Path.home", + return_value=root.parent, + ): + result = _detect_dev_source() + assert result != root, ( + "Gate must require the exact string '1' to avoid ambiguous " + "truthy values silently re-opening the hole." + ) + + +class TestBootstrapFindDevSourceSecurityHardening: + """The bootstrap script runs as a subprocess of the primary handler + and inherits the parent's environment. Its ``_find_dev_source`` MUST + apply the same gate — otherwise a future call site that invokes the + bootstrap directly (or transitively) would re-introduce the rsync + overwrite path that GHSA-gvpp-v77h-5w8g identified as a secondary + code-execution surface. + """ + + def test_claude_project_dir_is_ignored(self, monkeypatch): + with tempfile.TemporaryDirectory(prefix="cortex-bootstrap-malicious-") as td: + attacker_root = Path(td) + _plant_marker_files(attacker_root) + monkeypatch.delenv("CORTEX_DEV_SOURCE_SYNC", raising=False) + monkeypatch.delenv("CORTEX_DEV_ROOT", raising=False) + monkeypatch.setenv("CLAUDE_PROJECT_DIR", str(attacker_root)) + with patch( + "mcp_server.server.visualize_bootstrap.Path.home", + return_value=attacker_root.parent, + ): + result = bootstrap_find_dev_source() + assert result is None, ( + f"CLAUDE_PROJECT_DIR should be ignored by bootstrap; got {result!r} — " + "regression of GHSA-gvpp-v77h-5w8g secondary path." + ) + + def test_cortex_dev_root_ignored_without_opt_in(self, monkeypatch): + with tempfile.TemporaryDirectory(prefix="cortex-bootstrap-unopted-") as td: + attacker_root = Path(td) + _plant_marker_files(attacker_root) + monkeypatch.delenv("CORTEX_DEV_SOURCE_SYNC", raising=False) + monkeypatch.setenv("CORTEX_DEV_ROOT", str(attacker_root)) + monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) + with patch( + "mcp_server.server.visualize_bootstrap.Path.home", + return_value=attacker_root.parent, + ): + result = bootstrap_find_dev_source() + assert result is None, ( + f"Bootstrap honoured CORTEX_DEV_ROOT without the opt-in flag; " + f"got {result!r}." + ) + + def test_cortex_dev_root_honoured_when_opted_in(self, monkeypatch): + with tempfile.TemporaryDirectory(prefix="cortex-bootstrap-legit-") as td: + dev_root = Path(td) + _plant_marker_files(dev_root) + monkeypatch.setenv("CORTEX_DEV_SOURCE_SYNC", "1") + monkeypatch.setenv("CORTEX_DEV_ROOT", str(dev_root)) + monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) + with patch( + "mcp_server.server.visualize_bootstrap.Path.home", + return_value=dev_root.parent, + ): + result = bootstrap_find_dev_source() + assert result == dev_root, ( + f"Bootstrap legitimate dev workflow broken: expected {dev_root!r}, " + f"got {result!r}." + )