[WEB-5059] fix(claude-code): resolve mcpServers array form in plugin hook

[anonpran]

Closes WEB-5059.

Problem

Per the Claude Code plugin spec, plugin.json/.mcp.json mcpServers is typed string | array | object ("MCP config paths or inline config"). The plugin pretool hook (_plugin_mcp_server_map) handled the string (single path), object (inline), and unwrapped-root forms — but not the array-of-paths form:

{ "mcpServers": ["./mcp/a.json", "./mcp/b.json"] }

A list matched neither the str nor dict branch → silently fell through → those servers were never read → the gateway fingerprinted the bare name to null → false-block (sanction fail-open, same class as the Playwright/Forter prod bug).

Fix

Extracted the string-path resolver (with its resolve() + relative_to() traversal-containment check) into _load_plugin_mcp_map, and reuse it for both the string branch and a new list branch (each element resolved + merged via setdefault). String / object / unwrapped-root behavior is unchanged.

Tests (claude-code/hooks/test_pretool_mcp.py)

• array of 2 paths → both servers resolve
• ../ array element → rejected (containment)
• mixed valid + escaping element → valid one still resolves, bad one dropped

57/57 pass. Client-hook only; no gateway/backend changes.

Validation note

The plugin spec declares the array type but ships no literal array example, and no public plugin was found using it — so this is spec-completeness / defensive (validated by unit tests). Follow-up to setup#203 (unwrapped .mcp.json).

🤖 Generated with Claude Code

---

Note

Low Risk
Client-side hook change only; reuses existing path containment checks and is covered by new unit tests with no gateway changes.

Overview
Fixes plugin MCP resolution when mcpServers is an array of config paths — a spec-supported form that was previously ignored, so those servers never reached the gateway fingerprint and could be falsely blocked.

Refactors single-path loading into _load_plugin_mcp_map (same resolve() + containment rules as the string case) and adds a list branch that loads each path, merges server entries (first key wins; logs conflicting duplicates). Inline object and unwrapped .mcp.json behavior is unchanged.

New unit tests cover multi-file resolution, ../ rejection in arrays, and keeping valid paths when another array element escapes.

^{Reviewed by Cursor Bugbot for commit 6b84ab8. Bugbot is set up for automated code reviews on this repo. Configure here.}

Greptile Summary

This PR fixes a gap in the Claude Code plugin pretool hook where mcpServers specified as an array of config paths (e.g., ["./a.json", "./b.json"]) was silently ignored — causing those servers to never load and the gateway to fingerprint them as null, producing false-blocks. The string-path resolver is refactored into _load_plugin_mcp_map (containment/symlink checks preserved) and reused for both the existing string branch and a new list branch.

• _load_plugin_mcp_map (new helper): resolves each path relative to version_dir, rejects absolute paths and ../ traversal via resolve() + relative_to(), and reads the target file's mcpServers dict.
• Array branch in _plugin_mcp_server_map: iterates array elements through the new helper, merges with first-wins (setdefault), and logs duplicate-key collisions for diagnosability.
• Tests: three new cases cover multi-path resolution, traversal-escape rejection, and partial array validity (bad element dropped, good one kept).

Confidence Score: 5/5

Safe to merge — client-side hook change only, no gateway or auth impact, containment guards preserved.

The refactor is mechanically sound: path containment is checked per-element, the existing string and inline-object paths are unchanged, and three new tests cover the boundary conditions. The only open items are minor debuggability gaps that do not affect correctness or security.

No files require special attention.

Important Files Changed

Filename	Overview
claude-code/hooks/unbound.py	Extracts string-path resolver into _load_plugin_mcp_map and adds an isinstance(mcp_servers, list) branch; containment/symlink guards are preserved and applied per-element, with collision logging.
claude-code/hooks/test_pretool_mcp.py	Adds three new tests for the array form: multi-path resolution, ../ traversal rejection, and mixed valid/escaping elements; tests are isolated and correctly exercise the containment boundary.

_{Reviews (2): Last reviewed commit: "fix(claude-code): log array-key collisio..." | Re-trigger Greptile}

[vigneshsubbiah16]

🛡️ Automated Security Review (consensus)

2 findings — 1 high-confidence, 1 to triage. Reviewers: Lead, Claude, Semgrep, Gitleaks.

First-wins array merge may fingerprint wrong MCP server

claude-code/hooks/unbound.py:745
Impact: When multiple array-listed config files define the same server key, setdefault keeps the first entry and silently drops later ones; if the Claude Code client uses last-wins merge semantics, the hook can report a different URL/command than the client actually connects to, causing sanction checks against the wrong server (fail-open class of hook-vs-client divergence).
Fix: Confirm the client's array merge order and match it; at minimum log_error on duplicate keys so collisions are observable.
Reviewers: Claude, Greptile

Silent containment rejections leave no audit trail

claude-code/hooks/unbound.py:697
Impact: _load_plugin_mcp_map returns {} with no logging when a path is absolute, escapes via ../, or resolves outside the version dir via symlink—legitimate misconfigs degrade to null-fingerprint false-blocks with nothing in logs to diagnose, and malicious plugin probing leaves no trace.
Fix: Add log_error(f"mcp plugin path rejected (containment): {rel_path}", 'mcp_plugin') on the ValueError path.
Reviewers: Claude

---

Note: Semgrep flagged pre-existing insecure-file-permissions at unbound.py:1929 and :2162; those lines are outside this PR's diff and were not re-raised. Gitleaks reported no secrets. Path containment logic (resolve() + relative_to()) is sound and correctly extended to array elements.

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 35377e28 · 2026-07-18T13:31Z

[anonpran]

Both review findings addressed in 6b84ab8 (observability — no change to resolution outcomes):

• 🔴 First-wins array merge may fingerprint wrong server (unbound.py:745) — the collision this worries about only occurs if two array-listed files define the same server key (a pathological override pattern; the normal array use is distinct servers across files). Claude Code's array merge order is undocumented (the spec ships no array example), so rather than guess it I kept first-wins — consistent with the existing cross-source setdefault — and now log_error on a genuine key conflict, so any hook↔client divergence is diagnosable instead of silent.
• 🟡 Silent containment rejections leave no audit trail (unbound.py:697) — added log_error(f"mcp plugin path rejected (containment): {rel_path}", 'mcp_plugin') on the reject path, so an absolute/..//symlink-escape path is now traceable (aids false-block diagnosis + surfaces malicious plugin probing).

57/57 tests pass.

[vigneshsubbiah16]

✅ Security consensus: no issues found. (reviewers: Cursor, Claude [unavailable], Semgrep, Gitleaks)

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 6b84ab8c · 2026-07-19T17:00Z