From 35377e28b0ad21bb8b9230f0289941ada377c92a Mon Sep 17 00:00:00 2001 From: Nanda Pranesh Date: Sat, 18 Jul 2026 18:50:53 +0530 Subject: [PATCH 1/2] fix(claude-code): resolve `mcpServers` array form in plugin hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the plugin spec `mcpServers` is `string | array | object`. The plugin hook handled the string (single path), object (inline), and unwrapped-root forms but not the array-of-paths form — a list fell through both branches, so those servers were never read → the gateway fingerprinted the bare name to null → false-block (sanction fail-open, same class as the Playwright/Forter bug). Extract the string-path resolver (with its traversal-containment check) into `_load_plugin_mcp_map` and reuse it for both the string branch and a new list branch. String / object / unwrapped-root behavior is unchanged. Closes WEB-5059. Co-Authored-By: Claude Opus 4.8 (1M context) --- claude-code/hooks/test_pretool_mcp.py | 45 +++++++++++++++++++++++++ claude-code/hooks/unbound.py | 48 ++++++++++++++++++--------- 2 files changed, 77 insertions(+), 16 deletions(-) diff --git a/claude-code/hooks/test_pretool_mcp.py b/claude-code/hooks/test_pretool_mcp.py index d6b17ea..ae15908 100644 --- a/claude-code/hooks/test_pretool_mcp.py +++ b/claude-code/hooks/test_pretool_mcp.py @@ -146,6 +146,51 @@ def test_string_form_absolute_path_rejected(self): cfg = unbound._resolve_plugin_mcp_config("plugin_absplug_evil", cache_dir=self.cache) self.assertIsNone(cfg) + def test_array_form_resolves_all_servers(self): + # `mcpServers` as an array of config paths must resolve every listed file. + _make_plugin( + self.cache, "mkt", "toolkit", "1.0.0", + { + ".claude-plugin/plugin.json": {"mcpServers": ["./a.json", "./b.json"]}, + "a.json": {"mcpServers": {"alpha": {"url": "https://alpha.example/mcp"}}}, + "b.json": {"mcpServers": {"beta": {"url": "https://beta.example/mcp"}}}, + }, + ) + self.assertEqual( + unbound._resolve_plugin_mcp_config("plugin_toolkit_alpha", cache_dir=self.cache), + {"url": "https://alpha.example/mcp"}, + ) + self.assertEqual( + unbound._resolve_plugin_mcp_config("plugin_toolkit_beta", cache_dir=self.cache), + {"url": "https://beta.example/mcp"}, + ) + + def test_array_form_traversal_escape_rejected(self): + # A ../ element in the array must not read a file outside the version dir. + ver_dir = _make_plugin( + self.cache, "mkt", "arrevil", "1.0.0", + {".claude-plugin/plugin.json": {"mcpServers": ["../../../evil.json"]}}, + ) + outside = ver_dir.parent.parent.parent / "evil.json" + _write_json(outside, {"mcpServers": {"evil": {"url": "https://evil.example/mcp"}}}) + self.assertIsNone( + unbound._resolve_plugin_mcp_config("plugin_arrevil_evil", cache_dir=self.cache) + ) + + def test_array_form_drops_bad_element_keeps_valid(self): + # One escaping element must not poison the rest of the array. + _make_plugin( + self.cache, "mkt", "mixed", "1.0.0", + { + ".claude-plugin/plugin.json": {"mcpServers": ["./ok.json", "../../../evil.json"]}, + "ok.json": {"mcpServers": {"good": {"url": "https://good.example/mcp"}}}, + }, + ) + self.assertEqual( + unbound._resolve_plugin_mcp_config("plugin_mixed_good", cache_dir=self.cache), + {"url": "https://good.example/mcp"}, + ) + def test_identical_config_collision_resolves(self): # Two distinct (plugin, server) pairs mangle to the same candidate AND # share the SAME config -> one distinct entry -> resolves (benign). diff --git a/claude-code/hooks/unbound.py b/claude-code/hooks/unbound.py index 3e3c3c8..8c17f35 100644 --- a/claude-code/hooks/unbound.py +++ b/claude-code/hooks/unbound.py @@ -685,6 +685,29 @@ def _norm_mcp_token(s: Optional[str]) -> str: return re.sub(r'_+', '_', s or '').strip('_') +def _load_plugin_mcp_map(version_dir: Path, rel_path) -> Dict: + # Read a plugin-relative config file's `mcpServers` map. Contain the path to + # the version dir: reject absolute paths and ../ traversal (and symlink + # escapes via resolve()). + if not isinstance(rel_path, str): + return {} + candidate = (version_dir / rel_path).resolve() + try: + candidate.relative_to(version_dir.resolve()) + except ValueError: + return {} + if not candidate.is_file(): + return {} + try: + with open(candidate, 'r', encoding='utf-8') as f: + data = json.loads(f.read()) + except Exception as exc: + log_error(f"mcp plugin source unreadable: {candidate}: {exc}", 'mcp_plugin') + return {} + servers = data.get('mcpServers') if isinstance(data, dict) else None + return servers if isinstance(servers, dict) else {} + + def _plugin_mcp_server_map(version_dir: Path) -> Dict: servers = {} sources = [version_dir / ".mcp.json", version_dir / ".claude-plugin" / "plugin.json"] @@ -711,23 +734,16 @@ def _plugin_mcp_server_map(version_dir: Path) -> Dict: } if root_map: mcp_servers = root_map + # Per the plugin spec `mcpServers` is `string | array | object`: a config + # path, a list of config paths, or an inline map. Resolve paths to maps. if isinstance(mcp_servers, str): - # Contain the path to the version dir: reject absolute paths and - # ../ traversal (and symlink escapes via resolve()). - candidate = (version_dir / mcp_servers).resolve() - try: - candidate.relative_to(version_dir.resolve()) - except ValueError: - continue - if candidate.is_file(): - try: - with open(candidate, 'r', encoding='utf-8') as f: - rel_data = json.loads(f.read()) - except Exception as exc: - log_error(f"mcp plugin source unreadable: {candidate}: {exc}", 'mcp_plugin') - continue - if isinstance(rel_data, dict): - mcp_servers = rel_data.get('mcpServers') + mcp_servers = _load_plugin_mcp_map(version_dir, mcp_servers) + elif isinstance(mcp_servers, list): + merged = {} + for elem in mcp_servers: + for key, entry in _load_plugin_mcp_map(version_dir, elem).items(): + merged.setdefault(key, entry) + mcp_servers = merged if isinstance(mcp_servers, dict): for key, entry in mcp_servers.items(): servers.setdefault(key, entry) From 6b84ab8c16352aa257cfd7a78f74fe1303b808a4 Mon Sep 17 00:00:00 2001 From: Nanda Pranesh Date: Sun, 19 Jul 2026 22:25:32 +0530 Subject: [PATCH 2/2] fix(claude-code): log array-key collisions + containment rejects Address PR #225 review (observability): - Log a real array-key collision in the plugin `mcpServers` array merge, so a wrong-server fingerprint from a first-wins/last-wins divergence is diagnosable instead of silent. Kept first-wins (consistent with the cross-source merge). - Log a path-containment rejection in `_load_plugin_mcp_map` so an absolute/../ /symlink-escape path leaves an audit trail (aids false-block diagnosis and surfaces malicious plugin probing). No change to resolution outcomes; observability only. Co-Authored-By: Claude Opus 4.8 (1M context) --- claude-code/hooks/unbound.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/claude-code/hooks/unbound.py b/claude-code/hooks/unbound.py index 8c17f35..3f409a7 100644 --- a/claude-code/hooks/unbound.py +++ b/claude-code/hooks/unbound.py @@ -695,6 +695,7 @@ def _load_plugin_mcp_map(version_dir: Path, rel_path) -> Dict: try: candidate.relative_to(version_dir.resolve()) except ValueError: + log_error(f"mcp plugin path rejected (containment): {rel_path}", 'mcp_plugin') return {} if not candidate.is_file(): return {} @@ -739,9 +740,14 @@ def _plugin_mcp_server_map(version_dir: Path) -> Dict: if isinstance(mcp_servers, str): mcp_servers = _load_plugin_mcp_map(version_dir, mcp_servers) elif isinstance(mcp_servers, list): + # First-wins on a duplicate key (consistent with the cross-source + # setdefault below); log a real conflict so a wrong-server fingerprint + # is diagnosable rather than silent. merged = {} for elem in mcp_servers: for key, entry in _load_plugin_mcp_map(version_dir, elem).items(): + if key in merged and merged[key] != entry: + log_error(f"mcp plugin array key collision: {key}", 'mcp_plugin') merged.setdefault(key, entry) mcp_servers = merged if isinstance(mcp_servers, dict):