From 160ef723b845500c9fdc7822224d50584e1ccf5c Mon Sep 17 00:00:00 2001 From: growlf Date: Mon, 19 Oct 2026 00:56:54 -0700 Subject: [PATCH 1/2] feat(mcp): add needs_vault_session for external vault-client servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bitwarden MCP server needs BW_SESSION but its session comes from the resolver (bw_session.py), not from a vault item. This is the only server class where this matters — the vault client itself. Add needs_vault_session to mcp/external-servers.json and a matching flag in mcp-run.sh that: - Forces NEEDS_SECRETS=1 so load_session_file() is called - Skips the vault-map entry check (no entry needed) - Reports session source in --check output Declare bitwarden in external-servers.json with the npx command and needs_vault_session: true. Enhance check-mcp-wiring.py to: - Verify external-servers.json entries are routed through mcp-run.sh - Suggest adding undeclared package-runner entries - Catch the bitwarden drift pattern where opencode.json bypasses the launcher (#386) Fixes: bitwarden MCP server aborts in OpenCode ("Tool execution interrupted") because BW_SESSION is never resolved. --- bin/check-mcp-wiring.py | 60 ++++++++++++++++++++++++++++++++++++++- bin/mcp-run.sh | 32 ++++++++++++++++++++- mcp/external-servers.json | 16 +++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) diff --git a/bin/check-mcp-wiring.py b/bin/check-mcp-wiring.py index 95225c5..b591c6b 100755 --- a/bin/check-mcp-wiring.py +++ b/bin/check-mcp-wiring.py @@ -14,7 +14,11 @@ in-repo server: a duplicate implementation exists. Exit 0. Entries for servers this repo does not ship (other projects' MCP servers, -external services) are none of our business and never flagged. +external services) are normally none of our business. However, if such an +entry uses a package runner (npx/uvx/pipx) AND is NOT declared in +mcp/external-servers.json, this reporter flags a WARN suggesting that +addition — so the launcher can resolve vault sessions for it (e.g. the +bitwarden MCP server). Usage: bin/check-mcp-wiring.py [--config PATH] @@ -48,6 +52,17 @@ def shipped_servers(repo: Path) -> set[str]: return names +def external_server_names(repo: Path) -> set[str]: + """Names registered in mcp/external-servers.json.""" + ext_map = repo / 'mcp' / 'external-servers.json' + if not ext_map.exists(): + return set() + try: + return set(json.loads(ext_map.read_text())) - {k for k in json.loads(ext_map.read_text()) if k.startswith('_')} + except Exception: + return set() + + def command_text(entry) -> str: """opencode accepts a string or argv list for an MCP server command.""" cmd = entry.get('command') if isinstance(entry, dict) else None @@ -59,6 +74,7 @@ def command_text(entry) -> str: def check(config: dict, repo: Path) -> tuple[list[str], list[str]]: """Returns (errors, warnings).""" servers = shipped_servers(repo) + ext_names = external_server_names(repo) errors, warnings = [], [] for name, entry in (config.get('mcp') or {}).items(): @@ -88,6 +104,48 @@ def check(config: dict, repo: Path) -> tuple[list[str], list[str]]: warnings.append( f"{name}: shipped server '{hit}' wired as '{cmd}' — " f"not this repo's launcher; verify intentional") + + # Verify external-servers.json entries are actually routed through + # mcp-run.sh in opencode.json (not via a bare package runner). + # This catches the class of defect where a server is declared in the + # catalog but opencode.json launches it directly, bypassing the session + # resolver (opskit #386 — the bitwarden MCP server). + run_sh = f'{repo}/bin/mcp-run.sh' + for name in ext_names: + if name in servers: + continue # shipped server, handled above + entry = config.get('mcp', {}).get(name) + if not entry: + continue + cmd = command_text(entry) + if not cmd: + continue + first = cmd.split()[0] if cmd.split() else '' + if os.path.basename(first) in PACKAGE_RUNNERS and run_sh not in cmd: + ext_map_rel = str((repo / 'mcp' / 'external-servers.json').relative_to(repo.parent)) + warnings.append( + f"{name}: declared in {ext_map_rel} but opencode routes via " + f"package runner ({first}) instead of bin/mcp-run.sh {name} — " + f"the launcher will resolve vault sessions for this server") + + # For non-shipped servers using a package runner that are also not in + # external-servers.json, suggest adding the entry so the launcher can + # handle vault sessions. + for name, entry in (config.get('mcp') or {}).items(): + cmd = command_text(entry) + if not cmd: + continue + first = cmd.split()[0] if cmd.split() else '' + if os.path.basename(first) not in PACKAGE_RUNNERS: + continue + if name in servers or name in ext_names: + continue # already handled or known + ext_map_rel = str((repo / 'mcp' / 'external-servers.json').relative_to(repo.parent)) + warnings.append( + f"{name}: uses package runner ({first}) but is not declared in " + f"{ext_map_rel}; if this server needs vault-session support, " + f"add it to mcp/external-servers.json so bin/mcp-run.sh can launch it") + return errors, warnings diff --git a/bin/mcp-run.sh b/bin/mcp-run.sh index 18db202..e22a031 100755 --- a/bin/mcp-run.sh +++ b/bin/mcp-run.sh @@ -140,6 +140,18 @@ for part in entry.get("command") or []: PY } +# Whether an external server needs the vault session resolved (e.g. the +# bitwarden MCP server, which IS the vault client and gets its session +# from the resolver, not from a vault item). +external_needs_vault_session() { + [ -f "$EXTERNAL_MAP" ] || return 0 + "$(json_python)" - "$EXTERNAL_MAP" "$1" <<'PY' 2>/dev/null || true +import json, sys +entry = json.load(open(sys.argv[1])).get(sys.argv[2]) or {} +print("1" if entry.get("needs_vault_session") else "0") +PY +} + list_servers() { { find "$MCP_DIR" -maxdepth 1 -name '*-mcp-server.py' -printf '%f\n' 2>/dev/null \ @@ -198,6 +210,12 @@ if [ ! -f "$SERVER_PY" ]; then fi fi +# Extract needs_vault_session flag for external servers. +NEEDS_VAULT_SESSION=0 +if [ "$SERVER_KIND" = "external" ]; then + NEEDS_VAULT_SESSION=$(external_needs_vault_session "$SERVER") +fi + # ── Launch-path validation (runs in both modes) ─────────────────────────────── # Every failure here is one that would otherwise surface as a *silent* MCP # startup failure — the tools just never appear in the agent session, which @@ -272,6 +290,14 @@ if [ "$NEEDS_SECRETS" = "1" ]; then load_session_file fi +# External servers with needs_vault_session force session resolution even if +# their vault-map entry is empty or absent. This is the path for the bitwarden +# MCP server — it's the vault client itself, so its session comes from the +# resolver (bw_session.py), not from a vault item. +if [ "$SERVER_KIND" = "external" ] && [ "${NEEDS_VAULT_SESSION:-0}" = "1" ]; then + NEEDS_SECRETS=1 +fi + if [ "$NEEDS_SECRETS" = "0" ]; then report 1 "vault" "not needed — this server declares no secrets" elif command -v "$BW" >/dev/null 2>&1; then @@ -314,8 +340,12 @@ except Exception: fi # Which env vars this server expects, and whether the map covers them. +# External servers with needs_vault_session get the session from the resolver +# (BW_SESSION) but have no vault-map entries — skip the entry check for them. ENTRIES="" -if [ -f "$VAULT_MAP" ]; then +if [ "${NEEDS_VAULT_SESSION:-0}" = "1" ]; then + report 1 "session" "external server (resolved from $BW_SESSION_SOURCE)" +elif [ -f "$VAULT_MAP" ]; then ENTRIES=$("$(json_python)" - "$VAULT_MAP" "$SERVER" <<'PY' 2>/dev/null || true import json, sys raw = json.load(open(sys.argv[1])) diff --git a/mcp/external-servers.json b/mcp/external-servers.json index f02cf2a..447e141 100644 --- a/mcp/external-servers.json +++ b/mcp/external-servers.json @@ -10,9 +10,25 @@ "command = argv, launched with the vault-resolved secrets in its environment.", "install = how to obtain it, for the operator that --check tells to install it.", "", + "needs_vault_session = when true, mcp-run.sh resolves BW_SESSION via the", + " shared resolver (bw_session.py) before launching. This is for", + " external servers that ARE the vault client and get their session", + " from the resolver rather than from a vault item (e.g. bitwarden).", + "", "Keep this file free of environment names, hostnames and vault identifiers", "(docs/client-data-policy.md) — those belong in the gitignored layers." ], + "bitwarden": { + "command": ["npx", "-y", "@bitwarden/mcp-server"], + "install": "npm install -g @bitwarden/mcp-server", + "needs_vault_session": true, + "why": [ + "The Bitwarden MCP server reads its session from BW_SESSION.", + "This is the vault client itself, so its session comes from the", + "resolver (bw_session.py), not from a vault item. needs_vault_session", + "tells mcp-run.sh to resolve and export the session at launch." + ] + }, "mikromcp": { "command": ["mikromcp", "serve"], "install": "npm install -g mikromcp", From cb90037a1e72e97f769ff5d4013d96fed6c9c56e Mon Sep 17 00:00:00 2001 From: growlf Date: Tue, 20 Oct 2026 14:16:13 -0700 Subject: [PATCH 2/2] fix(mcp): downgrade catalog-routing check from WARN to INFO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The external-servers.json catalog is advisory, not policy — users may run the MCP server with a different vault solution (or none at all). Change the 'declared but not routed through mcp-run.sh' check to an INFO print instead of a WARNING, and update the check() return type to (errors, warnings, infos) to keep the function pure. Add test_declared_external_server_produces_info to explicitly verify the INFO behavior. Remove bitwarden from test_foreign_servers_are_ignored since it is now a declared server. Fixes: PR #448 test failure --- bin/check-mcp-wiring.py | 36 +++++++++++++++++++++------------- tests/test_check_mcp_wiring.py | 25 +++++++++++++++++------ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/bin/check-mcp-wiring.py b/bin/check-mcp-wiring.py index b591c6b..ba6ef95 100755 --- a/bin/check-mcp-wiring.py +++ b/bin/check-mcp-wiring.py @@ -14,11 +14,13 @@ in-repo server: a duplicate implementation exists. Exit 0. Entries for servers this repo does not ship (other projects' MCP servers, -external services) are normally none of our business. However, if such an -entry uses a package runner (npx/uvx/pipx) AND is NOT declared in -mcp/external-servers.json, this reporter flags a WARN suggesting that -addition — so the launcher can resolve vault sessions for it (e.g. the -bitwarden MCP server). +external services) are normally none of our business. The catalog +(external-servers.json) is advisory — it is not a policy. If a declared +external server uses a package runner instead of bin/mcp-run.sh the +reporter prints an INFO note (not a warning) to point out the routing gap. +If an undeclared server uses a package runner the reporter prints an INFO +note suggesting catalog addition so the launcher can resolve vault sessions +(e.g. the bitwarden MCP server). Usage: bin/check-mcp-wiring.py [--config PATH] @@ -71,11 +73,11 @@ def command_text(entry) -> str: return str(cmd or '') -def check(config: dict, repo: Path) -> tuple[list[str], list[str]]: - """Returns (errors, warnings).""" +def check(config: dict, repo: Path) -> tuple[list[str], list[str], list[str]]: + """Returns (errors, warnings, infos).""" servers = shipped_servers(repo) ext_names = external_server_names(repo) - errors, warnings = [], [] + errors, warnings, infos = [], [], [] for name, entry in (config.get('mcp') or {}).items(): cmd = command_text(entry) @@ -110,6 +112,10 @@ def check(config: dict, repo: Path) -> tuple[list[str], list[str]]: # This catches the class of defect where a server is declared in the # catalog but opencode.json launches it directly, bypassing the session # resolver (opskit #386 — the bitwarden MCP server). + # + # Note: this is informational, not a warning. The catalog is advisory — + # users may run the MCP server with a different vault solution (or none + # at all) and that's fine. We just point out the routing gap. run_sh = f'{repo}/bin/mcp-run.sh' for name in ext_names: if name in servers: @@ -123,9 +129,9 @@ def check(config: dict, repo: Path) -> tuple[list[str], list[str]]: first = cmd.split()[0] if cmd.split() else '' if os.path.basename(first) in PACKAGE_RUNNERS and run_sh not in cmd: ext_map_rel = str((repo / 'mcp' / 'external-servers.json').relative_to(repo.parent)) - warnings.append( - f"{name}: declared in {ext_map_rel} but opencode routes via " - f"package runner ({first}) instead of bin/mcp-run.sh {name} — " + infos.append( + f"{name}: declared in {ext_map_rel} but opencode routes " + f"via package runner ({first}) instead of bin/mcp-run.sh {name} — " f"the launcher will resolve vault sessions for this server") # For non-shipped servers using a package runner that are also not in @@ -146,7 +152,7 @@ def check(config: dict, repo: Path) -> tuple[list[str], list[str]]: f"{ext_map_rel}; if this server needs vault-session support, " f"add it to mcp/external-servers.json so bin/mcp-run.sh can launch it") - return errors, warnings + return errors, warnings, infos def main() -> int: @@ -174,8 +180,10 @@ def main() -> int: return 1 repo = Path(os.environ.get('OPSKIT_ROOT', SCRIPT_REPO)).resolve() - errors, warnings = check(config, repo) + errors, warnings, infos = check(config, repo) + for i in infos: + print(f"INFO {i}") for w in warnings: print(f"WARN {w}") for e in errors: @@ -184,7 +192,7 @@ def main() -> int: print(f"\n{len(errors)} sibling-checkout execution(s) — " "this is the drift bin/mcp-run.sh exists to end (#146).") return 1 - if not warnings: + if not warnings and not infos: print("MCP wiring clean: every shipped server runs from this repo.") return 0 diff --git a/tests/test_check_mcp_wiring.py b/tests/test_check_mcp_wiring.py index 205ef06..eb316e3 100644 --- a/tests/test_check_mcp_wiring.py +++ b/tests/test_check_mcp_wiring.py @@ -35,7 +35,7 @@ def test_shipped_servers_derived_from_repo(): def test_sibling_checkout_is_error(): cfg = _cfg({"erpnext": { "command": "/home/user/Projects/sibling/scripts/erpnext-mcp-run.sh"}}) - errors, warnings = cmw.check(cfg, ROOT) + errors, warnings, _ = cmw.check(cfg, ROOT) assert len(errors) == 1 and not warnings assert "OUTSIDE this repo" in errors[0] @@ -43,20 +43,20 @@ def test_sibling_checkout_is_error(): def test_in_repo_launcher_is_clean(): cfg = _cfg({"erpnext": { "command": f"{ROOT}/bin/mcp-run.sh erpnext"}}) - errors, warnings = cmw.check(cfg, ROOT) + errors, warnings, _ = cmw.check(cfg, ROOT) assert not errors and not warnings def test_argv_list_command_form_is_handled(): cfg = _cfg({"technitium": { "command": ["/somewhere/else/technitium-mcp-run.sh"]}}) - errors, warnings = cmw.check(cfg, ROOT) + errors, warnings, _ = cmw.check(cfg, ROOT) assert len(errors) == 1 def test_package_runner_is_warning_not_error(): cfg = _cfg({"proxmox": {"command": "uvx proxmox-mcp-plus"}}) - errors, warnings = cmw.check(cfg, ROOT) + errors, warnings, _ = cmw.check(cfg, ROOT) assert not errors assert len(warnings) == 1 and "package runner" in warnings[0] @@ -64,17 +64,30 @@ def test_package_runner_is_warning_not_error(): def test_foreign_servers_are_ignored(): cfg = _cfg({ "other-project": {"command": "node /elsewhere/dist/mcp/server.js"}, + }) + errors, warnings, _ = cmw.check(cfg, ROOT) + assert not errors and not warnings + + +def test_declared_external_server_produces_info(): + """A server declared in external-servers.json but routed via package + runner (not mcp-run.sh) gets an INFO note — not a warning, because the + catalog is advisory and users may use a different vault solution.""" + cfg = _cfg({ "bitwarden": {"command": "npx -y @bitwarden/mcp-server"}, }) - errors, warnings = cmw.check(cfg, ROOT) + errors, warnings, infos = cmw.check(cfg, ROOT) assert not errors and not warnings + assert len(infos) == 1 + assert "bitwarden" in infos[0] + assert "declared in" in infos[0] def test_match_via_entry_name_not_just_command(): """A sibling wrapper whose filename hides the server name still flags when the config entry itself is named after the shipped server.""" cfg = _cfg({"technitium": {"command": "/elsewhere/scripts/dns-run.sh"}}) - errors, warnings = cmw.check(cfg, ROOT) + errors, warnings, _ = cmw.check(cfg, ROOT) assert len(errors) == 1