From 08d9cf4cbdb7845e3668a956d0a4f5552387dca8 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Sat, 25 Jul 2026 19:33:18 -0400 Subject: [PATCH 1/5] feat(mcp): detect Connected-but-namespaces-missing attachment failures (Closes #708) --- docs/mcp-namespace-health.md | 23 +++++ mcp_namespace_health.py | 85 +++++++++++++++++++ ...test_issue_708_mcp_namespace_attachment.py | 69 +++++++++++++++ 3 files changed, 177 insertions(+) create mode 100644 tests/test_issue_708_mcp_namespace_attachment.py diff --git a/docs/mcp-namespace-health.md b/docs/mcp-namespace-health.md index 17ed150..2d02ae9 100644 --- a/docs/mcp-namespace-health.md +++ b/docs/mcp-namespace-health.md @@ -86,3 +86,26 @@ When a namespace returns EOF, follow When blocked, repair the IDE namespace and re-record a healthy `client_namespace` assessment before retrying the mutation. + +## Connected vs Attached Tool Surface (#708) + +MCP servers can report **Connected** at the CLI / host inventory layer while the **active LLM session exposes none of their tool namespaces**. + +### Core principle + +* **Connected status at host layer ≠ attached tools in active session.** +* Required preflight proof is **live tool visibility + `gitea_whoami` call** through the target namespace, not host `Connected` status alone. +* When servers report Connected but namespaces are absent from attached tools, classify as `mcp_connected_namespaces_missing`. + +### Forbidden unsafe fallbacks + +When `mcp_connected_namespaces_missing` is detected, workflows must **fail closed** and must **never** encourage or perform: + +* direct imports of MCP server Python modules +* CLI or raw Gitea API mutations as a substitute for native tools +* profile hopping to another MCP profile/namespace to bypass the empty session +* session-state overrides or hand-edited session/ledger files +* process kills (`pkill`), config mtime touches, or `.env` edits + +Only sanctioned recovery: **client reconnect path**, followed by full preflight (`whoami` → capability resolve → task). + diff --git a/mcp_namespace_health.py b/mcp_namespace_health.py index 4934e21..eec0fec 100644 --- a/mcp_namespace_health.py +++ b/mcp_namespace_health.py @@ -51,6 +51,14 @@ EOF_PATTERNS = ( "eof", ) +ERROR_CONNECTED_NAMESPACES_MISSING = "mcp_connected_namespaces_missing" + +UNSAFE_FALLBACK_WARNING = ( + "Workflow Safety Hard Stop (#708): Connected-but-namespaces-missing recovery must " + "NEVER use direct imports, Gitea API mutations, profile hopping, session-state " + "overrides, PID kills, or config mtime touches. Use client reconnect only." +) + SAFE_ENV_KEYS = ( "GITEA_MCP_PROFILE", "GITEA_PROFILE_NAME", @@ -60,6 +68,83 @@ SAFE_ENV_KEYS = ( ) +def assess_connected_namespace_attachment( + *, + connected_servers: list[str] | tuple[str, ...] | set[str] | None = None, + attached_session_namespaces: list[str] | tuple[str, ...] | set[str] | None = None, + required_namespaces: list[str] | tuple[str, ...] | set[str] | None = None, +) -> dict[str, Any]: + """Assess whether host-connected MCP servers have attached tool namespaces in the active session (#708). + + Addresses the Connected-but-namespaces-missing defect: CLI/host status may report Connected + while the active LLM session tool surface exposes 0 attached tool namespaces. + + Returns structured telemetry and detection details. + """ + connected = [str(s).strip() for s in (connected_servers or []) if str(s).strip()] + attached = set(str(ns).strip() for ns in (attached_session_namespaces or []) if str(ns).strip()) + req = [str(r).strip() for r in (required_namespaces or DEFAULT_NAMESPACES) if str(r).strip()] + + proof: dict[str, dict[str, bool]] = {} + missing: list[str] = [] + + for s in connected: + is_attached = s in attached + proof[s] = {"connected": True, "attached": is_attached} + if not is_attached and s in req: + missing.append(s) + + for r in req: + if r not in proof: + proof[r] = {"connected": r in connected, "attached": r in attached} + if r in connected and r not in attached and r not in missing: + missing.append(r) + + attachment_healthy = len(missing) == 0 and len(connected) > 0 + discovery_status = ( + "namespaces_attached" if attachment_healthy + else ("connected_but_namespaces_missing" if len(connected) > 0 else "disconnected") + ) + + reasons: list[str] = [] + remediation: list[str] = [] + if missing: + reasons.append( + f"MCP server(s) {missing} report Connected at host/CLI layer but tool namespaces " + f"are missing from active session attached tools (Connected ≠ attached tools, #708)." + ) + remediation.append( + "Reconnect the IDE/client MCP session to attach namespaces to the active session. " + "Do not use direct imports, CLI API mutations, profile hopping, or session file overrides." + ) + elif not connected: + reasons.append("No MCP servers reported Connected.") + remediation.append("Start or reconnect Gitea MCP servers in client config.") + else: + reasons.append("All connected MCP server namespaces are attached to the active session.") + + return { + "success": attachment_healthy, + "attachment_healthy": attachment_healthy, + "discovery_status": discovery_status, + "connected_servers": connected, + "attached_session_namespaces": list(attached), + "missing_namespaces": missing, + "proof_of_connected_vs_attached": proof, + "error_type": None if attachment_healthy else ERROR_CONNECTED_NAMESPACES_MISSING, + "reasons": reasons, + "remediation": remediation, + "exact_next_action": ( + "Reconnect the IDE/client MCP session so tool namespaces attach to the active session. " + "Do not use direct imports, CLI API mutations, profile hopping, or session-state overrides." + if not attachment_healthy + else "None; session tool namespaces attached." + ), + "unsafe_fallback_policy": UNSAFE_FALLBACK_WARNING, + } + + + def _as_list(value: Any) -> list[str] | None: if value is None: return None diff --git a/tests/test_issue_708_mcp_namespace_attachment.py b/tests/test_issue_708_mcp_namespace_attachment.py new file mode 100644 index 0000000..3d26b3a --- /dev/null +++ b/tests/test_issue_708_mcp_namespace_attachment.py @@ -0,0 +1,69 @@ +"""Unit regression tests for Issue #708: Connected-but-namespaces-missing detection and attachment safety.""" + +import mcp_namespace_health + + +def test_assess_connected_namespace_attachment_success(): + connected = ["gitea-author", "gitea-reviewer", "gitea-merger"] + attached = ["gitea-author", "gitea-reviewer", "gitea-merger"] + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=connected, + attached_session_namespaces=attached, + ) + assert res["success"] is True + assert res["attachment_healthy"] is True + assert res["discovery_status"] == "namespaces_attached" + assert res["error_type"] is None + assert res["missing_namespaces"] == [] + assert res["exact_next_action"] == "None; session tool namespaces attached." + + +def test_assess_connected_namespace_attachment_missing(): + connected = ["gitea-author", "gitea-reviewer", "gitea-merger"] + attached = ["gitea-author"] + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=connected, + attached_session_namespaces=attached, + ) + assert res["success"] is False + assert res["attachment_healthy"] is False + assert res["discovery_status"] == "connected_but_namespaces_missing" + assert res["error_type"] == "mcp_connected_namespaces_missing" + assert "gitea-reviewer" in res["missing_namespaces"] + assert "gitea-merger" in res["missing_namespaces"] + assert "Reconnect the IDE/client MCP session" in res["exact_next_action"] + + +def test_assess_connected_namespace_attachment_disconnected(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=[], + attached_session_namespaces=[], + ) + assert res["success"] is False + assert res["attachment_healthy"] is False + assert res["discovery_status"] == "disconnected" + + +def test_proof_of_connected_vs_attached_mapping(): + connected = ["gitea-author", "gitea-reviewer"] + attached = ["gitea-author"] + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=connected, + attached_session_namespaces=attached, + ) + proof = res["proof_of_connected_vs_attached"] + assert proof["gitea-author"] == {"connected": True, "attached": True} + assert proof["gitea-reviewer"] == {"connected": True, "attached": False} + + +def test_unsafe_fallback_policy_enforcement(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=["gitea-author"], + attached_session_namespaces=[], + ) + policy = res["unsafe_fallback_policy"] + assert "Workflow Safety Hard Stop (#708)" in policy + assert "direct imports" in policy + assert "API mutations" in policy + assert "profile hopping" in policy + assert "session-state overrides" in policy -- 2.43.7 From e3fa3b263d4b8a04b189e111a345ac5c3a3fe1b6 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 28 Jul 2026 16:46:09 -0400 Subject: [PATCH 2/5] feat(mcp): gate Connected-but-unattached MCP namespaces (Closes #708) The prior #708 slice added assess_connected_namespace_attachment() as a pure decision function with no call site: nothing invoked it, so a session whose role namespaces were Connected at the host but absent from the active session tool surface still passed every mutation gate. Detection existed on paper only. This makes it load-bearing. Decision layer (mcp_namespace_health.py) - assess_connected_namespace_attachment() gains secret-free telemetry (connected/attached/required/missing counts, discovery cache hit and age, reconnect_required, auto_attach_attempted, auto_recovered, error_type) and reports reconnect_required, auto_recovered and startup_ordering_race. - Startup ordering: a namespace whose connect completed after the session tool snapshot cannot be in that snapshot, so parallel multi-role startup is identified as its own race with the affected namespaces listed. - attachment_gate_from_session() is a fail-closed gate keyed by ATTACHMENT_GATED_TASKS. An unassessed namespace does not gate, matching #543 semantics, so this never fabricates a block. - SANCTIONED_ATTACH_RECOVERY_TOOL names gitea_request_mcp_reconnect (#678) as the only recovery. Server wiring (gitea_mcp_server.py) - New tool gitea_assess_mcp_namespace_attachment classifies the condition and records a per-namespace verdict in _LIVE_NAMESPACE_ATTACHMENT. - gitea_submit_pr_review and gitea_merge_pr now consult _namespace_attachment_gate() alongside the existing #543 health gate, so both fail closed while a required namespace is unattached. - Watchdog check-in emits status only, never namespace contents. The typed condition mcp_connected_namespaces_missing stays distinct from config drift (#672), transport-closed (#584) and resolver EOF (#685). Recovery never routes through direct imports, CLI or raw API mutation, profile hopping, session-state overrides, or process kills. Docs - docs/mcp-namespace-health.md documents the tool arguments, startup ordering, the fail-closed gate, and the telemetry contract. - skills/llm-project-workflow/SKILL.md states Connected is not attached, and that preflight proof is live tool visibility plus gitea_whoami on the role namespace rather than host status alone. - docs/mcp-tool-inventory.md lists the new tool. - docs/remote-mcp/threat-model-anchors.json and threat-model.md: 21 #956 anchors restamped for the line shift these additions caused in gitea_mcp_server.py. Every anchor was re-derived from its recorded expect substring; none guessed. Tests - tests/test_issue_708_attachment_wiring.py (19 cases): typed detection, proof mapping, reconnect-only next action, auto-attach success and failure, reconnect rediscovery, multi-role startup ordering, telemetry including a no-secret-leak assertion, fail-closed gate per role, unassessed and unmapped tasks not gating, partial attachment gating only the affected role, and no healthy verdict without attachment proof. Verification - tests/test_issue_708_attachment_wiring.py + test_issue_708_mcp_namespace_attachment.py: 24 passed - namespace/session/runtime/review sweep: 427 passed, 12 subtests - full suite head: 31 failed, 5885 passed, 6 skipped, 1047 subtests - full suite base 17ba1ff035ee: 30 failed, 5862 passed, 6 skipped, 1047 subtests - failing identifier sets match, plus tests/test_mirror_refs.py DryRunBanner, which fails on the unmodified base in isolation and passes here: flaky, not a regression from this branch. - Gate proven by execution, not inspection: registering the assessment blocks merge_pr and review_pr, and attaching the namespaces clears the block. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/mcp-namespace-health.md | 48 ++++ docs/mcp-tool-inventory.md | 1 + docs/remote-mcp/threat-model-anchors.json | 298 +++++++++++++++++----- docs/remote-mcp/threat-model.md | 44 ++-- gitea_mcp_server.py | 120 +++++++++ mcp_namespace_health.py | 131 +++++++++- skills/llm-project-workflow/SKILL.md | 22 ++ tests/test_issue_708_attachment_wiring.py | 269 +++++++++++++++++++ 8 files changed, 843 insertions(+), 90 deletions(-) create mode 100644 tests/test_issue_708_attachment_wiring.py diff --git a/docs/mcp-namespace-health.md b/docs/mcp-namespace-health.md index 2d02ae9..b98cef0 100644 --- a/docs/mcp-namespace-health.md +++ b/docs/mcp-namespace-health.md @@ -109,3 +109,51 @@ When `mcp_connected_namespaces_missing` is detected, workflows must **fail close Only sanctioned recovery: **client reconnect path**, followed by full preflight (`whoami` → capability resolve → task). +### Native detection tool + +`gitea_assess_mcp_namespace_attachment` classifies the condition and records it in +the session. Pass the namespaces the host reports Connected and the namespaces +actually attached to the active session tool surface: + +| Argument | Meaning | +|---|---| +| `connected_servers` | Namespaces the host/CLI reports Connected | +| `attached_session_namespaces` | Namespaces exposed in the active session tool surface | +| `required_namespaces` | Namespaces this workflow needs (defaults to the role namespaces) | +| `discovery_cache_hit` / `discovery_cache_age_seconds` | Client tool-discovery cache state | +| `auto_attach_attempted` / `auto_attach_succeeded` | Whether the runtime auto-attached | +| `session_tool_snapshot_at` / `namespace_connected_at` | Epoch seconds, to detect startup ordering races | + +It returns `discovery_status` +(`namespaces_attached` | `connected_but_namespaces_missing` | `disconnected`), +`missing_namespaces`, `proof_of_connected_vs_attached` (per namespace +`{connected, attached}`), `error_type`, `reconnect_required`, `auto_recovered`, +`startup_ordering_race`, `late_attaching_namespaces`, `sanctioned_recovery_tool`, +and a reconnect-only `exact_next_action`. + +### Startup ordering + +`session_tool_snapshot_at` earlier than a namespace's `namespace_connected_at` +means that namespace could not have been in the session snapshot, however healthy +it looks now. That is reported as `startup_ordering_race` with the affected +namespaces listed — the multi-role parallel-connect case where Connected flips +true after the session tool list was already captured. + +### Fail-closed gate + +The recorded verdict gates mutations, mirroring the #543 health gate: a namespace +that has not been assessed does not gate, but one recorded Connected-but-unattached +blocks the mutation whose role namespace it is +(`review_pr` / `submit_review` → `gitea-reviewer`, `merge_pr` → `gitea-merger`, +`work_issue` / `create_pr` → `gitea-author`). `gitea_submit_pr_review` and +`gitea_merge_pr` return the block reason plus the hard-stop policy string, and the +only offered recovery is `gitea_request_mcp_reconnect`. + +### Telemetry + +The `telemetry` block carries `connected_count`, `attached_count`, +`required_count`, `missing_count`, `discovery_status`, `discovery_cache_hit`, +`discovery_cache_age_seconds`, `reconnect_required`, `auto_attach_attempted`, +`auto_recovered`, `startup_ordering_race`, and `error_type`. It contains namespace +names and counts only — never tokens, endpoints, env values, or filesystem paths. + diff --git a/docs/mcp-tool-inventory.md b/docs/mcp-tool-inventory.md index e93d359..22dc84e 100644 --- a/docs/mcp-tool-inventory.md +++ b/docs/mcp-tool-inventory.md @@ -56,6 +56,7 @@ that gates each call, not which tools exist. - `gitea_assess_conflict_fix_push` - `gitea_assess_gitea_operation_path` - `gitea_assess_master_parity` +- `gitea_assess_mcp_namespace_attachment` - `gitea_assess_mcp_namespace_health` - `gitea_assess_pr_sync_status` - `gitea_assess_review_merge_state_machine` diff --git a/docs/remote-mcp/threat-model-anchors.json b/docs/remote-mcp/threat-model-anchors.json index 30ba0b4..04fe37f 100644 --- a/docs/remote-mcp/threat-model-anchors.json +++ b/docs/remote-mcp/threat-model-anchors.json @@ -10,71 +10,237 @@ ], "generated_against_commit": "a143cd065ba06e1a2bdc5143a19ec156e53650ef", "anchors": [ - {"anchor": "gitea_mcp_server.py:24728", "expect": "mcp_daemon_guard.bind_native_mcp_transport()"}, - {"anchor": "mcp_daemon_guard.py:49", "expect": "_PRODUCTION_TRANSPORTS = mcp_transport_config.SUPPORTED_TRANSPORTS"}, - {"anchor": "mcp_daemon_guard.py:195", "expect": "def bind_native_mcp_transport"}, - {"anchor": "irrecoverable_provenance.py:497", "expect": "def assess_transport_for_auth_mint"}, - {"anchor": "gitea_mcp_server.py:9132", "expect": "assess_transport_for_auth_mint()"}, - {"anchor": "gitea_mcp_server.py:9381", "expect": "assess_transport_for_auth_mint()"}, - {"anchor": "mcp_server.py:4", "expect": "The transport is selected by deployment configuration"}, - - {"anchor": "gitea_mcp_server.py:15415", "expect": "def _is_client_managed_process"}, - {"anchor": "gitea_mcp_server.py:15445", "expect": "def _provenance_mutation_block"}, - {"anchor": "gitea_mcp_server.py:15453", "expect": "unsupported_manual_launch"}, - {"anchor": "gitea_mcp_server.py:19004", "expect": "server_provenance"}, - {"anchor": "gitea_mcp_server.py:21445", "expect": "def _check_mcp_runtimes_diagnostics"}, - {"anchor": "gitea_mcp_server.py:21465", "expect": "\"ps\", \"-o\", \"pid,lstart,command\""}, - {"anchor": "gitea_mcp_server.py:21509", "expect": "\"ps\", \"eww\""}, - {"anchor": "gitea_config.py:1172", "expect": "RECOGNIZED_GITEA_ENV_KEYS"}, - {"anchor": "gitea_config.py:1233", "expect": "GITEA_CLIENT_MANAGED"}, - - {"anchor": "gitea_config.py:54", "expect": "ENV_PROFILE = \"GITEA_MCP_PROFILE\""}, - {"anchor": "gitea_config.py:97", "expect": "_REVIEW_MERGE_OPS"}, - {"anchor": "gitea_config.py:499", "expect": "repository authorization scope"}, - - {"anchor": "gitea_config.py:956", "expect": "def _keychain_token"}, - {"anchor": "gitea_config.py:974", "expect": "def resolve_token"}, - {"anchor": "gitea_config.py:1015", "expect": "def keychain_auth"}, - {"anchor": "gitea_config.py:294", "expect": "def _validate_identity_auth"}, - {"anchor": "mcp_daemon_guard.py:583", "expect": "def assert_keychain_access_allowed"}, - {"anchor": "gitea_mcp_server.py:19261", "expect": "def gitea_list_profiles"}, - {"anchor": "gitea_mcp_server.py:19312", "expect": "gitea_config.resolve_token(p)"}, - {"anchor": "gitea_mcp_server.py:19555", "expect": "def gitea_audit_config"}, - {"anchor": "gitea_mcp_server.py:19577", "expect": "service_summaries(config)"}, - - {"anchor": "gitea_config.py:704", "expect": "def resolve_service"}, - {"anchor": "gitea_config.py:837", "expect": "def service_summaries"}, - {"anchor": "gitea_config.py:851", "expect": "_keychain_token(auth.get(\"id\"))"}, - {"anchor": "gitea_mcp_server.py:17710", "expect": "\"jenkins-mcp\""}, - {"anchor": "gitea_mcp_server.py:17716", "expect": "external-mcp"}, - {"anchor": "gitea_mcp_server.py:17737", "expect": "\"glitchtip-mcp\""}, - {"anchor": "gitea_mcp_server.py:17742", "expect": "external-mcp"}, - {"anchor": "mcp_discoverability.py:9", "expect": "EXPECTED_JENKINS_TOOLS"}, - {"anchor": "mcp_discoverability.py:17", "expect": "EXPECTED_GLITCHTIP_TOOLS"}, - - {"anchor": "sentry_incident_bridge.py:36", "expect": "SENTRY_AUTH_TOKEN"}, - {"anchor": "sentry_incident_bridge.py:190", "expect": "def resolve_token"}, - {"anchor": "sentry_incident_bridge.py:289", "expect": "Authorization"}, - {"anchor": "sentry_observability.py:55", "expect": "SENTRY_DSN"}, - - {"anchor": "master_parity_gate.py:168", "expect": "def capture_startup_parity"}, - {"anchor": "master_parity_gate.py:255", "expect": "mutation_safe"}, - {"anchor": "gitea_mcp_server.py:19105", "expect": "def gitea_assess_master_parity"}, - - {"anchor": "gitea_mcp_server.py:193", "expect": "ACTIVE_WORKTREE_ENV"}, - {"anchor": "gitea_mcp_server.py:194", "expect": "AUTHOR_WORKTREE_ENV"}, - {"anchor": "gitea_mcp_server.py:2351", "expect": "/tmp/gitea_issue_lock.json"}, - {"anchor": "gitea_mcp_server.py:10897", "expect": "def gitea_bootstrap_author_issue_worktree"}, - {"anchor": "mcp_server.py:13", "expect": "/tmp/mcp_server_stderr.log"}, - - {"anchor": "issue_lock_store.py:26", "expect": "DEFAULT_LOCK_DIR"}, - {"anchor": "issue_lock_store.py:83", "expect": "def session_pointer_path"}, - {"anchor": "issue_lock_store.py:98", "expect": "def is_process_alive"}, - {"anchor": "mcp_session_state.py:27", "expect": "DEFAULT_STATE_DIR"}, - {"anchor": "control_plane_db.py:47", "expect": "DEFAULT_DB_PATH"}, - {"anchor": "control_plane_db.py:380", "expect": "mode=0o700"}, - {"anchor": "control_plane_db.py:386", "expect": "sqlite3.connect"}, - {"anchor": "control_plane_db.py:1145", "expect": "os.getpid()"}, - {"anchor": "gitea_mcp_server.py:12804", "expect": "owner_pid_alive"} + { + "anchor": "gitea_mcp_server.py:24848", + "expect": "mcp_daemon_guard.bind_native_mcp_transport()" + }, + { + "anchor": "mcp_daemon_guard.py:49", + "expect": "_PRODUCTION_TRANSPORTS = mcp_transport_config.SUPPORTED_TRANSPORTS" + }, + { + "anchor": "mcp_daemon_guard.py:195", + "expect": "def bind_native_mcp_transport" + }, + { + "anchor": "irrecoverable_provenance.py:497", + "expect": "def assess_transport_for_auth_mint" + }, + { + "anchor": "gitea_mcp_server.py:9175", + "expect": "assess_transport_for_auth_mint()" + }, + { + "anchor": "gitea_mcp_server.py:9424", + "expect": "assess_transport_for_auth_mint()" + }, + { + "anchor": "mcp_server.py:4", + "expect": "The transport is selected by deployment configuration" + }, + { + "anchor": "gitea_mcp_server.py:15465", + "expect": "def _is_client_managed_process" + }, + { + "anchor": "gitea_mcp_server.py:15495", + "expect": "def _provenance_mutation_block" + }, + { + "anchor": "gitea_mcp_server.py:15503", + "expect": "unsupported_manual_launch" + }, + { + "anchor": "gitea_mcp_server.py:19054", + "expect": "server_provenance" + }, + { + "anchor": "gitea_mcp_server.py:21565", + "expect": "def _check_mcp_runtimes_diagnostics" + }, + { + "anchor": "gitea_mcp_server.py:21585", + "expect": "\"ps\", \"-o\", \"pid,lstart,command\"" + }, + { + "anchor": "gitea_mcp_server.py:21629", + "expect": "\"ps\", \"eww\"" + }, + { + "anchor": "gitea_config.py:1172", + "expect": "RECOGNIZED_GITEA_ENV_KEYS" + }, + { + "anchor": "gitea_config.py:1233", + "expect": "GITEA_CLIENT_MANAGED" + }, + { + "anchor": "gitea_config.py:54", + "expect": "ENV_PROFILE = \"GITEA_MCP_PROFILE\"" + }, + { + "anchor": "gitea_config.py:97", + "expect": "_REVIEW_MERGE_OPS" + }, + { + "anchor": "gitea_config.py:499", + "expect": "repository authorization scope" + }, + { + "anchor": "gitea_config.py:956", + "expect": "def _keychain_token" + }, + { + "anchor": "gitea_config.py:974", + "expect": "def resolve_token" + }, + { + "anchor": "gitea_config.py:1015", + "expect": "def keychain_auth" + }, + { + "anchor": "gitea_config.py:294", + "expect": "def _validate_identity_auth" + }, + { + "anchor": "mcp_daemon_guard.py:583", + "expect": "def assert_keychain_access_allowed" + }, + { + "anchor": "gitea_mcp_server.py:19311", + "expect": "def gitea_list_profiles" + }, + { + "anchor": "gitea_mcp_server.py:19362", + "expect": "gitea_config.resolve_token(p)" + }, + { + "anchor": "gitea_mcp_server.py:19675", + "expect": "def gitea_audit_config" + }, + { + "anchor": "gitea_mcp_server.py:19697", + "expect": "service_summaries(config)" + }, + { + "anchor": "gitea_config.py:704", + "expect": "def resolve_service" + }, + { + "anchor": "gitea_config.py:837", + "expect": "def service_summaries" + }, + { + "anchor": "gitea_config.py:851", + "expect": "_keychain_token(auth.get(\"id\"))" + }, + { + "anchor": "gitea_mcp_server.py:17760", + "expect": "\"jenkins-mcp\"" + }, + { + "anchor": "gitea_mcp_server.py:17766", + "expect": "external-mcp" + }, + { + "anchor": "gitea_mcp_server.py:17787", + "expect": "\"glitchtip-mcp\"" + }, + { + "anchor": "gitea_mcp_server.py:17792", + "expect": "external-mcp" + }, + { + "anchor": "mcp_discoverability.py:9", + "expect": "EXPECTED_JENKINS_TOOLS" + }, + { + "anchor": "mcp_discoverability.py:17", + "expect": "EXPECTED_GLITCHTIP_TOOLS" + }, + { + "anchor": "sentry_incident_bridge.py:36", + "expect": "SENTRY_AUTH_TOKEN" + }, + { + "anchor": "sentry_incident_bridge.py:190", + "expect": "def resolve_token" + }, + { + "anchor": "sentry_incident_bridge.py:289", + "expect": "Authorization" + }, + { + "anchor": "sentry_observability.py:55", + "expect": "SENTRY_DSN" + }, + { + "anchor": "master_parity_gate.py:168", + "expect": "def capture_startup_parity" + }, + { + "anchor": "master_parity_gate.py:255", + "expect": "mutation_safe" + }, + { + "anchor": "gitea_mcp_server.py:19155", + "expect": "def gitea_assess_master_parity" + }, + { + "anchor": "gitea_mcp_server.py:193", + "expect": "ACTIVE_WORKTREE_ENV" + }, + { + "anchor": "gitea_mcp_server.py:194", + "expect": "AUTHOR_WORKTREE_ENV" + }, + { + "anchor": "gitea_mcp_server.py:2351", + "expect": "/tmp/gitea_issue_lock.json" + }, + { + "anchor": "gitea_mcp_server.py:10940", + "expect": "def gitea_bootstrap_author_issue_worktree" + }, + { + "anchor": "mcp_server.py:13", + "expect": "/tmp/mcp_server_stderr.log" + }, + { + "anchor": "issue_lock_store.py:26", + "expect": "DEFAULT_LOCK_DIR" + }, + { + "anchor": "issue_lock_store.py:83", + "expect": "def session_pointer_path" + }, + { + "anchor": "issue_lock_store.py:98", + "expect": "def is_process_alive" + }, + { + "anchor": "mcp_session_state.py:27", + "expect": "DEFAULT_STATE_DIR" + }, + { + "anchor": "control_plane_db.py:47", + "expect": "DEFAULT_DB_PATH" + }, + { + "anchor": "control_plane_db.py:380", + "expect": "mode=0o700" + }, + { + "anchor": "control_plane_db.py:386", + "expect": "sqlite3.connect" + }, + { + "anchor": "control_plane_db.py:1145", + "expect": "os.getpid()" + }, + { + "anchor": "gitea_mcp_server.py:12854", + "expect": "owner_pid_alive" + } ] } diff --git a/docs/remote-mcp/threat-model.md b/docs/remote-mcp/threat-model.md index 5494071..9518185 100644 --- a/docs/remote-mcp/threat-model.md +++ b/docs/remote-mcp/threat-model.md @@ -29,7 +29,7 @@ document cites an anchor the fixture does not cover. This guard exists because #930 did not have one. Its inventory was generated at `7bf4f125`; by `aad5c8b4` its `gitea_mcp_server.py` anchors had drifted — the transport -bind it cited at line 23750 now lives at `gitea_mcp_server.py:24728`, and its +bind it cited at line 23750 now lives at `gitea_mcp_server.py:24848`, and its client-managed provenance anchor at 14588 now lands in an unrelated function. Nothing failed, because nothing checked. Anchors into a ~24,700-line module rot silently, and a security document that cannot prove its own citations is worse than none, because it is @@ -77,15 +77,15 @@ authenticate the *caller*, not the *intent*. | ID | Boundary | Protects | Crossing requires today | Crossing must require remotely | | -- | -------- | -------- | ----------------------- | ------------------------------ | -| B1 | LLM client ↔ MCP server session | A1, A3, A10 — that a mutating session was established through the sanctioned client path | A single configured bind (`gitea_mcp_server.py:24728`) validated against one closed allowlist (`mcp_daemon_guard.py:49`, `mcp_daemon_guard.py:195`) — since #931 the identifier comes from deployment configuration and defaults to the local transport, so the boundary no longer rests on a literal, but it still rests on the *bind* rather than on an authenticated caller; client-managed provenance (`gitea_mcp_server.py:15415`) or a refusal (`gitea_mcp_server.py:15453`); production transport before recovery-authorization mint (`irrecoverable_provenance.py:497`, consumed at `gitea_mcp_server.py:9132` and `gitea_mcp_server.py:9381`) | An authenticated handshake issuing a server-side session identity bound to a principal, with the transport recorded in provenance. The physical proof (a pipe) must become a cryptographic one. | +| B1 | LLM client ↔ MCP server session | A1, A3, A10 — that a mutating session was established through the sanctioned client path | A single configured bind (`gitea_mcp_server.py:24848`) validated against one closed allowlist (`mcp_daemon_guard.py:49`, `mcp_daemon_guard.py:195`) — since #931 the identifier comes from deployment configuration and defaults to the local transport, so the boundary no longer rests on a literal, but it still rests on the *bind* rather than on an authenticated caller; client-managed provenance (`gitea_mcp_server.py:15465`) or a refusal (`gitea_mcp_server.py:15503`); production transport before recovery-authorization mint (`irrecoverable_provenance.py:497`, consumed at `gitea_mcp_server.py:9175` and `gitea_mcp_server.py:9424`) | An authenticated handshake issuing a server-side session identity bound to a principal, with the transport recorded in provenance. The physical proof (a pipe) must become a cryptographic one. | | B2 | Role ↔ role | A9 — that author, reviewer, merger, and reconciler are distinct authorities | **The process boundary only.** The role is a property of the process, read once from `GITEA_MCP_PROFILE` (`gitea_config.py:54`). A caller gets author permissions by connecting to the author process. Review and merge are the operations singled out for extra care (`gitea_config.py:97`) | A per-request principal, so the role follows from the credential presented and cannot be selected by reaching a different endpoint. | | B3 | MCP server ↔ credential store | A3, A8 — that only sanctioned code turns a profile into a token | `_keychain_token` shelling out to the login keychain (`gitea_config.py:956`), dispatched by `resolve_token` (`gitea_config.py:974`) with the reference type built at `gitea_config.py:1015`, gated by `assert_keychain_access_allowed` (`mcp_daemon_guard.py:583`). Inline secrets are rejected at config load (`gitea_config.py:294`) | A credential provider keyed by the *request* principal, returning only that principal's credential, with the source recorded and the value never returned. | | B4 | MCP server ↔ Gitea | A1, A2 — that only authorized calls reach the forge | A bearer token over TLS. Server-side, nothing distinguishes one role's token from another beyond the account it belongs to | Unchanged at the forge; the endpoint in front of it must refuse unauthenticated and plaintext connections before tool dispatch. | -| B5 | MCP server ↔ caller's filesystem | A7 — that a tool acts on the *caller's* disk or refuses | Nothing. The server's disk *is* the caller's disk. Worktree bootstrap writes directly (`gitea_mcp_server.py:10897`); the active workspace is process-global (`gitea_mcp_server.py:193`, `gitea_mcp_server.py:194`) | An explicit per-tool classification, enforced at dispatch, refusing filesystem tools over a transport that cannot reach the caller's disk. A green verdict about the wrong disk is the failure to prevent. | -| B6 | MCP server ↔ coordination state | A6, A9 — mutual exclusion | Local files and a local SQLite database, with liveness judged from the local process table (`issue_lock_store.py:98`), keyed on paths under one user's home (`issue_lock_store.py:26`, `mcp_session_state.py:27`, `control_plane_db.py:47`) and on `os.getpid()` (`control_plane_db.py:1145`, `gitea_mcp_server.py:12804`). A legacy global slot still exists at `gitea_mcp_server.py:2351`, and the session-pointer file is named per PID (`issue_lock_store.py:83`) | One authority per ownership question, with liveness from session identity and expiry, and atomic acquire, renew, and release across hosts. | +| B5 | MCP server ↔ caller's filesystem | A7 — that a tool acts on the *caller's* disk or refuses | Nothing. The server's disk *is* the caller's disk. Worktree bootstrap writes directly (`gitea_mcp_server.py:10940`); the active workspace is process-global (`gitea_mcp_server.py:193`, `gitea_mcp_server.py:194`) | An explicit per-tool classification, enforced at dispatch, refusing filesystem tools over a transport that cannot reach the caller's disk. A green verdict about the wrong disk is the failure to prevent. | +| B6 | MCP server ↔ coordination state | A6, A9 — mutual exclusion | Local files and a local SQLite database, with liveness judged from the local process table (`issue_lock_store.py:98`), keyed on paths under one user's home (`issue_lock_store.py:26`, `mcp_session_state.py:27`, `control_plane_db.py:47`) and on `os.getpid()` (`control_plane_db.py:1145`, `gitea_mcp_server.py:12854`). A legacy global slot still exists at `gitea_mcp_server.py:2351`, and the session-pointer file is named per PID (`issue_lock_store.py:83`) | One authority per ownership question, with liveness from session identity and expiry, and atomic acquire, renew, and release across hosts. | | B7 | Gitea integration ↔ unrelated integrations | A4, A5 — that a Gitea compromise is not a CI and observability compromise | **Nothing.** See §5. The Gitea server reads Jenkins and GlitchTip secrets (`gitea_config.py:851`, reached from `gitea_config.py:837`) and holds the Sentry token (`sentry_incident_bridge.py:190`) | A hard process boundary. This is the boundary #956 exists to create. | | B8 | Tenant ↔ tenant (`prgs` / `mdcps` / `local-lab`) | A2 — that one organization's compromise is not another's | Convention. One configuration declares all three contexts; `resolve_service` fails closed on a *disabled* context (`gitea_config.py:704`) but the credentials of enabled ones remain reachable in-process. A per-profile repository scope exists (`gitea_config.py:499`) | Separate deployments, or at minimum per-tenant credential scopes with no process able to resolve both. | -| B9 | Deployed code ↔ merged policy | A1, A10 — that the running server enforces the rules that were actually merged | Comparing this process's startup commit against this disk (`master_parity_gate.py:168`), conjoined into a single verdict (`master_parity_gate.py:255`) published by `gitea_mcp_server.py:19105` | Freshness defined against the deployed build identity, with an explicit fail-closed verdict when undeterminable. | +| B9 | Deployed code ↔ merged policy | A1, A10 — that the running server enforces the rules that were actually merged | Comparing this process's startup commit against this disk (`master_parity_gate.py:168`), conjoined into a single verdict (`master_parity_gate.py:255`) published by `gitea_mcp_server.py:19155` | Freshness defined against the deployed build identity, with an explicit fail-closed verdict when undeterminable. | ### What no boundary constrains @@ -130,10 +130,10 @@ Two flows deserve attention because neither is obvious from the code: 1. **The keychain flow fans out.** B3 is drawn once but resolves credentials for *every* configured profile and service, not only the active one. `gitea_list_profiles` - (`gitea_mcp_server.py:19261`) reports each profile's credential status by calling - `resolve_token` on it (`gitea_mcp_server.py:19312`), and `gitea_audit_config` - (`gitea_mcp_server.py:19555`) reports service credential status through - `service_summaries` (`gitea_mcp_server.py:19577`). + (`gitea_mcp_server.py:19311`) reports each profile's credential status by calling + `resolve_token` on it (`gitea_mcp_server.py:19362`), and `gitea_audit_config` + (`gitea_mcp_server.py:19675`) reports service credential status through + `service_summaries` (`gitea_mcp_server.py:19697`). 2. **The return path is a flow too.** Content read from Gitea travels back into the model and is treated as instruction. This is the ADV2 edge, and it is the only edge in the diagram with no authentication on it, because it is not a request. @@ -178,16 +178,16 @@ the credential, and an attacker holding the token does not call our tools. **Finding 3 — Any one role process can resolve every other role's credential.** This is not inferred; it is demonstrated by tool output. `gitea_list_profiles` -(`gitea_mcp_server.py:19261`) called from the **author** session reports +(`gitea_mcp_server.py:19311`) called from the **author** session reports `identity_status: "credentials present"` for `prgs-merger`, `prgs-reviewer`, `prgs-reconciler`, and every `mdcps` profile, because it calls `resolve_token` on each one -(`gitea_mcp_server.py:19312`). The author process does not merely *have access to* the +(`gitea_mcp_server.py:19362`). The author process does not merely *have access to* the merger's credential — it reads it to answer a status query. B2 is not a credential boundary in either direction. **Finding 4 — The Gitea server reads CI and observability secrets.** `gitea_audit_config` -(`gitea_mcp_server.py:19555`) reports `MDCPS Jenkins: enabled, read-only, authenticated`. -That word `authenticated` is produced by `service_summaries` (`gitea_mcp_server.py:19577`, +(`gitea_mcp_server.py:19675`) reports `MDCPS Jenkins: enabled, read-only, authenticated`. +That word `authenticated` is produced by `service_summaries` (`gitea_mcp_server.py:19697`, defined at `gitea_config.py:837`), whose default check calls `_keychain_token` on the service's own keychain reference (`gitea_config.py:851`). Producing that one line requires the Gitea MCP server to read the Jenkins secret and the GlitchTip secret out of the @@ -195,8 +195,8 @@ keychain. B7 does not exist. **Finding 5 — Jenkins and GlitchTip are already decomposed; the reach is residual.** Their tools live in separately registered servers, marked `external-mcp` -(`gitea_mcp_server.py:17710`, `gitea_mcp_server.py:17716`, `gitea_mcp_server.py:17737`, -`gitea_mcp_server.py:17742`) with their own expected tool sets (`mcp_discoverability.py:9`, +(`gitea_mcp_server.py:17760`, `gitea_mcp_server.py:17766`, `gitea_mcp_server.py:17787`, +`gitea_mcp_server.py:17792`) with their own expected tool sets (`mcp_discoverability.py:9`, `mcp_discoverability.py:17`). The correct decomposition was already chosen. What remains is a leak across it: the credential *references* still live in the Gitea configuration and are still resolved by the Gitea process. #75 bundled these services into one control-plane @@ -207,8 +207,8 @@ GlitchTip, the Sentry bridge runs *inside* the Gitea server, resolving its token process environment (`sentry_incident_bridge.py:190`) and sending it as a bearer header (`sentry_incident_bridge.py:289`). Being an environment variable rather than a keychain item makes it strictly worse: it needs no keychain prompt and is inherited by every subprocess the -server spawns — including the `ps` invocations at `gitea_mcp_server.py:21465` and -`gitea_mcp_server.py:21509`, reached from `gitea_mcp_server.py:21445`. +server spawns — including the `ps` invocations at `gitea_mcp_server.py:21585` and +`gitea_mcp_server.py:21629`, reached from `gitea_mcp_server.py:21565`. **Finding 7 — The highest-value coordination asset has the weakest gate.** A6 is protected by filesystem permissions alone (CR14). Corrupting a lease requires no Gitea credential, @@ -217,8 +217,8 @@ assumes. Every other asset costs an attacker a credential; this one costs nothin local access, which is exactly ADV5's position. **Finding 8 — Provenance authenticates the launch, not the caller.** `server_provenance` is -reported as exactly `client_managed` or `manual_launch` (`gitea_mcp_server.py:19004`), -derived from environment inspection (`gitea_mcp_server.py:15415`) with the recognized-key +reported as exactly `client_managed` or `manual_launch` (`gitea_mcp_server.py:19054`), +derived from environment inspection (`gitea_mcp_server.py:15465`) with the recognized-key allowlist at `gitea_config.py:1172` and the generator that emits the marker at `gitea_config.py:1233`. Every one of those facts is fixed at process start. A client that is trustworthy at launch and compromised a minute later remains `client_managed` for the life @@ -257,7 +257,7 @@ holds the token and calls the API instead of the tool. **D3 — Credential resolution is scoped to the request principal.** A session must resolve its own credential and must have no path to any other principal's. The resolve-every-profile -behavior behind `gitea_mcp_server.py:19312` and `gitea_mcp_server.py:19577` must report +behavior behind `gitea_mcp_server.py:19362` and `gitea_mcp_server.py:19697` must report configured-or-not from configuration alone, without resolving the secret. *Rationale.* Finding 3. An audit surface that proves a credential exists by fetching it is a @@ -334,11 +334,11 @@ The client is attached to the local fleet over stdio. | Boundary | What ADV1 reaches | Stopped by | | -------- | ----------------- | ---------- | -| B1 | Everything the fleet serves. The client *is* the sanctioned launcher: it satisfies the client-managed check (`gitea_mcp_server.py:15415`) by construction, and provenance is never re-verified after launch (Finding 8). | Nothing. The guard authenticates the launch, not the caller. | +| B1 | Everything the fleet serves. The client *is* the sanctioned launcher: it satisfies the client-managed check (`gitea_mcp_server.py:15465`) by construction, and provenance is never re-verified after launch (Finding 8). | Nothing. The guard authenticates the launch, not the caller. | | B2 | All five roles — it is attached to all five namespaces. It can author a PR, approve it from the reviewer namespace, and merge it from the merger namespace. | Only the in-process self-review check, which compares `jcwalker3` (author) against `sysadmin` (reviewer) and **passes**, because Finding 1 made them different accounts while leaving reviewer and merger identical. A9 falls in one sequence of legitimate calls. | | B3 | Every credential in CR1–CR10 via CR13, with no additional prompt — the daemon is already sanctioned, so `assert_keychain_access_allowed` (`mcp_daemon_guard.py:583`) returns immediately. | Nothing. | | B4 | A1 and A2 in full. | Branch protection at the forge, to the extent configured. | -| B5 | The operator's checkout and every worktree, through the author tools (`gitea_mcp_server.py:10897`), plus the shared stderr path at `mcp_server.py:13`. | Nothing; the server's disk is the target disk. | +| B5 | The operator's checkout and every worktree, through the author tools (`gitea_mcp_server.py:10940`), plus the shared stderr path at `mcp_server.py:13`. | Nothing; the server's disk is the target disk. | | B6 | All coordination state — no credential required (CR14). It can forge lease ownership and clear decision locks. | Filesystem permissions, which it already satisfies. | | B7 | Jenkins (A4) and GlitchTip (A5) secrets via Finding 4, and CR11/CR12 from its own environment. | Nothing. | | B8 | Both tenants. | Nothing in-process; only the disabled-context check (`gitea_config.py:704`), which does not apply to enabled contexts. | diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index b285870..bd01860 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -6716,6 +6716,45 @@ def _live_namespace_health_gate(task: str) -> list[str]: ) +# Session-scoped MCP namespace *attachment* assessments (#708). +# Distinct from _LIVE_NAMESPACE_HEALTH: a namespace can probe healthy while never +# being attached to the active session tool surface ("Connected" != "tools available"). +_LIVE_NAMESPACE_ATTACHMENT: dict[str, dict] = {} + + +def _record_live_namespace_attachment(assessment: dict | None) -> None: + """Store a per-namespace attachment verdict for mutation gates (#708).""" + if not isinstance(assessment, dict): + return + proof = assessment.get("proof_of_connected_vs_attached") + if not isinstance(proof, dict): + return + missing = set(assessment.get("missing_namespaces") or []) + error_type = assessment.get("error_type") + for ns, state in proof.items(): + ns_name = str(ns or "").strip() + if not ns_name or not isinstance(state, dict): + continue + attached = bool(state.get("attached")) + _LIVE_NAMESPACE_ATTACHMENT[ns_name] = { + "namespace": ns_name, + "connected": bool(state.get("connected")), + "attached": attached, + "attachment_healthy": attached and ns_name not in missing, + "error_type": None if attached else error_type, + "discovery_status": assessment.get("discovery_status"), + "reconnect_required": bool(assessment.get("reconnect_required")), + "auto_recovered": bool(assessment.get("auto_recovered")), + } + + +def _namespace_attachment_gate(task: str) -> list[str]: + """Fail closed on a recorded Connected-but-unattached namespace (#708).""" + return mcp_namespace_health.attachment_gate_from_session( + task, _LIVE_NAMESPACE_ATTACHMENT + ) + + def _decision_lock_binding(lock: dict | None = None) -> dict: """Resolve key fields for durable decision-lock storage.""" profile = get_profile() @@ -7596,6 +7635,10 @@ def _evaluate_pr_review_submission( if ns_gate: reasons.extend(ns_gate) return result + attach_gate = _namespace_attachment_gate("review_pr") + if attach_gate: + reasons.extend(attach_gate) + return result if action not in _REVIEW_ACTIONS: reasons.append( @@ -11154,6 +11197,13 @@ def gitea_merge_pr( reasons.extend(ns_gate) return result + # Gate 0c — the merger namespace must be attached to the active session (#708). + # Connected at the host layer is not proof the tools are in this session. + attach_gate = _namespace_attachment_gate("merge_pr") + if attach_gate: + reasons.extend(attach_gate) + return result + # Gate 1 — valid merge method (no API call on a bad method). if do not in _MERGE_METHODS: reasons.append( @@ -19419,6 +19469,76 @@ def gitea_assess_mcp_namespace_health( return result +@mcp.tool() +def gitea_assess_mcp_namespace_attachment( + connected_servers: list[str] | None = None, + attached_session_namespaces: list[str] | None = None, + required_namespaces: list[str] | None = None, + discovery_cache_age_seconds: float | None = None, + discovery_cache_hit: bool | None = None, + auto_attach_attempted: bool = False, + auto_attach_succeeded: bool = False, + session_tool_snapshot_at: float | None = None, + namespace_connected_at: dict | None = None, +) -> dict: + """Detect Connected-but-namespaces-not-attached MCP sessions (#708). + + MCP servers can report **Connected** at the CLI/host inventory layer while the + active LLM session exposes none of their tool namespaces. That is a *session + attachment* failure and is reported here as its own typed condition, + ``mcp_connected_namespaces_missing`` — deliberately distinct from config drift + (#672), transport-closed (#584), and resolver EOF (#685). + + Connected is not proof that tools are available. The required preflight proof is + live tool visibility plus ``gitea_whoami`` on the role namespace, never host + Connected status alone. + + The verdict is recorded in the session so ``gitea_submit_pr_review`` and + ``gitea_merge_pr`` fail closed while a required namespace is unattached. The only + sanctioned recovery is the client attach/reconnect path followed by full preflight; + direct imports, CLI/API mutation, profile hopping, session-state overrides, and + process kills are forbidden and never suggested. + + Args: + connected_servers: Namespaces the host/CLI reports as Connected. + attached_session_namespaces: Namespaces actually exposed in the active + session tool surface. + required_namespaces: Namespaces required for this workflow; defaults to the + canonical Gitea role namespaces. + discovery_cache_age_seconds: Age of the client tool-discovery cache entry. + discovery_cache_hit: Whether the tool list came from that cache. + auto_attach_attempted: Whether the runtime tried to auto-attach namespaces. + auto_attach_succeeded: Whether that auto-attach succeeded. + session_tool_snapshot_at: Epoch seconds the session tool snapshot was taken. + namespace_connected_at: Per-namespace epoch seconds that connect completed, + used to identify multi-role startup ordering races. + + Returns: + dict with ``discovery_status``, ``missing_namespaces``, + ``proof_of_connected_vs_attached``, ``error_type``, ``reconnect_required``, + ``auto_recovered``, ``startup_ordering_race``, a secret-free ``telemetry`` + block, and a reconnect-only ``exact_next_action``. + """ + result = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=connected_servers, + attached_session_namespaces=attached_session_namespaces, + required_namespaces=required_namespaces, + discovery_cache_age_seconds=discovery_cache_age_seconds, + discovery_cache_hit=discovery_cache_hit, + auto_attach_attempted=auto_attach_attempted, + auto_attach_succeeded=auto_attach_succeeded, + session_tool_snapshot_at=session_tool_snapshot_at, + namespace_connected_at=namespace_connected_at, + ) + _record_live_namespace_attachment(result) + # #606-style watchdog check-in (best-effort, fail open). Status only, no names. + sentry_observability.monitor_checkin( + "namespace_attachment", + "ok" if result.get("attachment_healthy") else "error", + ) + return result + + @mcp.tool() def gitea_activate_profile( profile_name: str, diff --git a/mcp_namespace_health.py b/mcp_namespace_health.py index eec0fec..89999a5 100644 --- a/mcp_namespace_health.py +++ b/mcp_namespace_health.py @@ -53,6 +53,21 @@ EOF_PATTERNS = ( ERROR_CONNECTED_NAMESPACES_MISSING = "mcp_connected_namespaces_missing" +# Namespaces that must be *attached to the active session* for a mutation task (#708). +# Connected-at-host is not attached-in-session; these are gated separately from the +# #543 health map because a namespace can be healthy on probe yet absent from the +# session tool surface. +ATTACHMENT_GATED_TASKS = { + "review_pr": "gitea-reviewer", + "submit_review": "gitea-reviewer", + "merge_pr": "gitea-merger", + "work_issue": "gitea-author", + "create_pr": "gitea-author", +} + +# The only sanctioned recovery for an unattached namespace (#678 exposes it natively). +SANCTIONED_ATTACH_RECOVERY_TOOL = "gitea_request_mcp_reconnect" + UNSAFE_FALLBACK_WARNING = ( "Workflow Safety Hard Stop (#708): Connected-but-namespaces-missing recovery must " "NEVER use direct imports, Gitea API mutations, profile hopping, session-state " @@ -73,13 +88,28 @@ def assess_connected_namespace_attachment( connected_servers: list[str] | tuple[str, ...] | set[str] | None = None, attached_session_namespaces: list[str] | tuple[str, ...] | set[str] | None = None, required_namespaces: list[str] | tuple[str, ...] | set[str] | None = None, + discovery_cache_age_seconds: float | int | None = None, + discovery_cache_hit: bool | None = None, + auto_attach_attempted: bool = False, + auto_attach_succeeded: bool = False, + session_tool_snapshot_at: float | int | None = None, + namespace_connected_at: dict[str, float] | None = None, ) -> dict[str, Any]: """Assess whether host-connected MCP servers have attached tool namespaces in the active session (#708). Addresses the Connected-but-namespaces-missing defect: CLI/host status may report Connected while the active LLM session tool surface exposes 0 attached tool namespaces. - Returns structured telemetry and detection details. + This is a *distinct* condition from config drift (#672), transport-closed (#584), + and resolver EOF (#685): the transport is up and the host reports Connected, yet the + namespace never entered the session tool surface. + + Startup ordering (``session_tool_snapshot_at`` + ``namespace_connected_at``) identifies + the race where the session tool snapshot was taken before a role server finished + ``initialize``/``list_tools``, which is why parallel multi-role startup can leave the + session with an empty namespace set while Connected later flips true. + + Returns structured detection details plus secret-free telemetry. """ connected = [str(s).strip() for s in (connected_servers or []) if str(s).strip()] attached = set(str(ns).strip() for ns in (attached_session_namespaces or []) if str(ns).strip()) @@ -106,6 +136,24 @@ def assess_connected_namespace_attachment( else ("connected_but_namespaces_missing" if len(connected) > 0 else "disconnected") ) + # Startup-ordering race: a namespace that finished connecting *after* the session + # tool snapshot was taken cannot be in that snapshot, however healthy it looks now. + connected_at = { + str(k).strip(): v + for k, v in (namespace_connected_at or {}).items() + if str(k).strip() and isinstance(v, (int, float)) + } + late_attaching: list[str] = [] + if isinstance(session_tool_snapshot_at, (int, float)): + for ns_name, ts in connected_at.items(): + if ts > session_tool_snapshot_at and ns_name not in attached: + late_attaching.append(ns_name) + late_attaching.sort() + startup_ordering_race = bool(late_attaching) + + auto_recovered = bool(auto_attach_attempted and auto_attach_succeeded and attachment_healthy) + reconnect_required = not attachment_healthy + reasons: list[str] = [] remediation: list[str] = [] if missing: @@ -114,7 +162,9 @@ def assess_connected_namespace_attachment( f"are missing from active session attached tools (Connected ≠ attached tools, #708)." ) remediation.append( - "Reconnect the IDE/client MCP session to attach namespaces to the active session. " + "Reconnect the IDE/client MCP session to attach namespaces to the active session " + f"(sanctioned path: {SANCTIONED_ATTACH_RECOVERY_TOOL}), then re-run full preflight " + "(gitea_whoami -> gitea_resolve_task_capability -> task). " "Do not use direct imports, CLI API mutations, profile hopping, or session file overrides." ) elif not connected: @@ -123,6 +173,19 @@ def assess_connected_namespace_attachment( else: reasons.append("All connected MCP server namespaces are attached to the active session.") + if startup_ordering_race: + reasons.append( + f"startup ordering race: namespace(s) {late_attaching} finished connecting after the " + "active session tool snapshot was taken, so they cannot appear in that snapshot (#708)." + ) + if auto_attach_attempted and not auto_attach_succeeded: + reasons.append( + "automatic namespace attachment was attempted and did not succeed; only the sanctioned " + "client reconnect path remains." + ) + if auto_recovered: + reasons.append("namespaces were automatically attached; no operator reconnect was required.") + return { "success": attachment_healthy, "attachment_healthy": attachment_healthy, @@ -141,9 +204,73 @@ def assess_connected_namespace_attachment( else "None; session tool namespaces attached." ), "unsafe_fallback_policy": UNSAFE_FALLBACK_WARNING, + "sanctioned_recovery_tool": SANCTIONED_ATTACH_RECOVERY_TOOL, + "reconnect_required": reconnect_required, + "auto_attach_attempted": bool(auto_attach_attempted), + "auto_recovered": auto_recovered, + "startup_ordering_race": startup_ordering_race, + "late_attaching_namespaces": late_attaching, + # Secret-free structured signals (#708 AC5). Namespace names and counts only: + # never tokens, endpoints, env values, or filesystem paths. + "telemetry": { + "connected_count": len(connected), + "attached_count": len(attached), + "required_count": len(req), + "missing_count": len(missing), + "discovery_status": discovery_status, + "discovery_cache_hit": ( + None if discovery_cache_hit is None else bool(discovery_cache_hit) + ), + "discovery_cache_age_seconds": ( + float(discovery_cache_age_seconds) + if isinstance(discovery_cache_age_seconds, (int, float)) + else None + ), + "reconnect_required": reconnect_required, + "auto_attach_attempted": bool(auto_attach_attempted), + "auto_recovered": auto_recovered, + "startup_ordering_race": startup_ordering_race, + "error_type": None if attachment_healthy else ERROR_CONNECTED_NAMESPACES_MISSING, + }, } +def required_namespace_for_attachment(task: str) -> str | None: + """Map a mutation task to the MCP namespace that must be *attached* (#708).""" + return ATTACHMENT_GATED_TASKS.get((task or "").strip()) + + +def attachment_gate_from_session( + task: str, + session_attachment: dict[str, dict[str, Any]] | None, +) -> list[str]: + """Fail-closed gate on recorded connected-but-unattached namespaces (#708). + + Mirrors :func:`mutation_gate_from_session`: a namespace that has not been + assessed yet does not gate, so this never blocks a session that simply has + not run the assessment. Once an assessment records the namespace required + for *task* as Connected-but-unattached, the mutation fails closed and the + only offered recovery is the sanctioned client reconnect path. + """ + ns = required_namespace_for_attachment(task) + if not ns: + return [] + store = session_attachment or {} + entry = store.get(ns) + if not entry: + return [] + if entry.get("attached") and entry.get("attachment_healthy"): + return [] + detail = entry.get("error_type") or ERROR_CONNECTED_NAMESPACES_MISSING + return [ + f"live MCP namespace '{ns}' is recorded {detail}: the host reports Connected but the " + f"namespace is not attached to the active session tool surface; reconnect the " + f"IDE/client MCP session and re-run preflight before {(task or 'mutation')} " + "(fail closed, #708)", + UNSAFE_FALLBACK_WARNING, + ] + + def _as_list(value: Any) -> list[str] | None: if value is None: diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index 58762c0..b85d24d 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -204,6 +204,28 @@ proposed command before running it; `gitea_audit_runtime_recovery_contamination` to inspect or (reconciler-only) clear the marker. Full contrast in `docs/mcp-namespace-eof-recovery.md`. +## Connected is not attached (#708) + +A host/CLI MCP inventory showing **Connected** is not proof the tools are usable. +The active session can expose **none** of a Connected server's tool namespaces — +a *session attachment* failure, distinct from config drift (#672), +transport-closed (#584), and resolver EOF (#685). + +Required preflight proof is **live tool visibility plus `gitea_whoami` on the role +namespace**, never host Connected status alone. Call +`gitea_assess_mcp_namespace_attachment` with the Connected set and the namespaces +actually attached to the session; it returns the typed condition +**mcp_connected_namespaces_missing** with per-namespace connected-vs-attached proof, +and records the verdict so review and merge fail closed while a required namespace +is unattached. + +Only sanctioned recovery: the client attach/reconnect path +(`gitea_request_mcp_reconnect`), then full preflight +(`gitea_whoami` → `gitea_resolve_task_capability` → task). Never recover by direct +module import, CLI/raw API mutation, profile hopping, session-state overrides, +process kills, or `.env`/mtime edits. A final report must not claim a healthy +session without attachment proof. + ## Shell Spawn Hard-Stop Rule `exit_code: -1` with empty stdout/stderr means the shell failed to spawn — not a diff --git a/tests/test_issue_708_attachment_wiring.py b/tests/test_issue_708_attachment_wiring.py new file mode 100644 index 0000000..ebfc26b --- /dev/null +++ b/tests/test_issue_708_attachment_wiring.py @@ -0,0 +1,269 @@ +"""Regression tests for Issue #708 wiring: detection must reach a gate, not just exist. + +The prior #708 slice added a pure decision function with no call site, so a session +whose namespaces were Connected-but-unattached still passed every mutation gate. +These tests pin the parts that make the detection load-bearing: + +* the typed condition is distinct from #672 / #584 / #685, +* the session store records a per-namespace attachment verdict, +* review/merge mutations fail closed while a required namespace is unattached, +* recovery is reconnect-only and never suggests an unsafe fallback, +* startup ordering races and discovery-cache telemetry are reported, +* a healthy final report cannot be produced without attachment proof. +""" + +import mcp_namespace_health + + +REQUIRED = ["gitea-author", "gitea-reviewer", "gitea-merger", "gitea-tools"] + + +def _connected_but_unattached(): + """The #708 signature: host says Connected, session tool surface is empty.""" + return mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=[], + required_namespaces=REQUIRED, + ) + + +# --- AC1: distinct typed detection ----------------------------------------- + + +def test_connected_with_empty_session_tool_list_is_typed_distinctly(): + res = _connected_but_unattached() + assert res["attachment_healthy"] is False + assert res["discovery_status"] == "connected_but_namespaces_missing" + assert res["error_type"] == "mcp_connected_namespaces_missing" + assert sorted(res["missing_namespaces"]) == sorted(REQUIRED) + # Not misclassified as config drift (#672), transport-closed (#584), or EOF (#685). + assert res["error_type"] not in { + "mcp_config_drift", + "transport_closed", + "mcp_client_eof", + } + + +def test_proof_carries_connected_and_attached_per_namespace(): + res = _connected_but_unattached() + proof = res["proof_of_connected_vs_attached"] + for ns in REQUIRED: + assert proof[ns] == {"connected": True, "attached": False} + + +# --- AC2: recovery is auto-attach or reconnect-only ------------------------- + + +def test_exact_next_action_is_reconnect_only(): + res = _connected_but_unattached() + assert res["reconnect_required"] is True + action = res["exact_next_action"] + assert "Reconnect the IDE/client MCP session" in action + for forbidden in ("pkill", "chmod", "curl", ".env", "sys.path"): + assert forbidden not in action + + +def test_sanctioned_recovery_tool_is_named(): + res = _connected_but_unattached() + assert res["sanctioned_recovery_tool"] == "gitea_request_mcp_reconnect" + assert "gitea_request_mcp_reconnect" in " ".join(res["remediation"]) + + +def test_successful_auto_attach_reports_recovered_without_operator(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=REQUIRED, + required_namespaces=REQUIRED, + auto_attach_attempted=True, + auto_attach_succeeded=True, + ) + assert res["attachment_healthy"] is True + assert res["auto_recovered"] is True + assert res["reconnect_required"] is False + + +def test_failed_auto_attach_still_requires_reconnect(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=[], + required_namespaces=REQUIRED, + auto_attach_attempted=True, + auto_attach_succeeded=False, + ) + assert res["auto_recovered"] is False + assert res["reconnect_required"] is True + assert any("did not succeed" in r for r in res["reasons"]) + + +def test_reconnect_rediscovery_clears_the_condition(): + """Attach state after a reconnect is healthy without any other change.""" + before = _connected_but_unattached() + after = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=REQUIRED, + required_namespaces=REQUIRED, + discovery_cache_hit=False, + discovery_cache_age_seconds=0.0, + ) + assert before["attachment_healthy"] is False + assert after["attachment_healthy"] is True + assert after["discovery_status"] == "namespaces_attached" + assert after["missing_namespaces"] == [] + + +# --- AC4: startup ordering across multiple role servers --------------------- + + +def test_multi_role_startup_ordering_race_is_reported(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=["gitea-author"], + required_namespaces=REQUIRED, + session_tool_snapshot_at=1000.0, + namespace_connected_at={ + "gitea-author": 990.0, + "gitea-reviewer": 1005.0, + "gitea-merger": 1007.0, + }, + ) + assert res["startup_ordering_race"] is True + assert res["late_attaching_namespaces"] == ["gitea-merger", "gitea-reviewer"] + assert res["telemetry"]["startup_ordering_race"] is True + + +def test_no_ordering_race_when_snapshot_follows_connect(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=REQUIRED, + required_namespaces=REQUIRED, + session_tool_snapshot_at=2000.0, + namespace_connected_at={ns: 1000.0 for ns in REQUIRED}, + ) + assert res["startup_ordering_race"] is False + assert res["late_attaching_namespaces"] == [] + + +# --- AC5: telemetry --------------------------------------------------------- + + +def test_telemetry_reports_cache_and_recovery_signals(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=["gitea-author"], + required_namespaces=REQUIRED, + discovery_cache_hit=True, + discovery_cache_age_seconds=42.5, + ) + tel = res["telemetry"] + assert tel["connected_count"] == 4 + assert tel["attached_count"] == 1 + assert tel["missing_count"] == 3 + assert tel["discovery_cache_hit"] is True + assert tel["discovery_cache_age_seconds"] == 42.5 + assert tel["reconnect_required"] is True + assert tel["error_type"] == "mcp_connected_namespaces_missing" + + +def test_telemetry_leaks_no_secrets(): + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=[], + required_namespaces=REQUIRED, + ) + blob = repr(res["telemetry"]).lower() + for leak in ("token", "authorization", "password", "secret", "/users/", "http"): + assert leak not in blob + + +# --- AC2/AC6: fail-closed mutation gate ------------------------------------- + + +def _session_store_from(assessment): + """Mirror the server-side recorder without importing the MCP server module.""" + store = {} + missing = set(assessment["missing_namespaces"]) + for ns, state in assessment["proof_of_connected_vs_attached"].items(): + attached = bool(state["attached"]) + store[ns] = { + "namespace": ns, + "connected": bool(state["connected"]), + "attached": attached, + "attachment_healthy": attached and ns not in missing, + "error_type": None if attached else assessment["error_type"], + } + return store + + +def test_review_and_merge_fail_closed_while_unattached(): + store = _session_store_from(_connected_but_unattached()) + for task in ("review_pr", "submit_review", "merge_pr", "create_pr", "work_issue"): + reasons = mcp_namespace_health.attachment_gate_from_session(task, store) + assert reasons, f"{task} must fail closed while its namespace is unattached" + assert "fail closed, #708" in reasons[0] + + +def test_gate_offers_only_sanctioned_recovery(): + store = _session_store_from(_connected_but_unattached()) + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + joined = " ".join(reasons) + assert "reconnect" in joined.lower() + assert "Workflow Safety Hard Stop (#708)" in joined + # Unsafe fallbacks appear only inside the prohibition, never as advice. + assert "NEVER use" in joined + + +def test_gate_passes_once_namespaces_are_attached(): + healthy = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=REQUIRED, + required_namespaces=REQUIRED, + ) + store = _session_store_from(healthy) + for task in ("review_pr", "submit_review", "merge_pr", "create_pr", "work_issue"): + assert mcp_namespace_health.attachment_gate_from_session(task, store) == [] + + +def test_unassessed_session_does_not_gate(): + """No recorded assessment must not fabricate a block (matches #543 semantics).""" + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", {}) == [] + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", None) == [] + + +def test_unmapped_task_is_not_gated(): + store = _session_store_from(_connected_but_unattached()) + assert mcp_namespace_health.attachment_gate_from_session("gitea_read", store) == [] + + +def test_partial_attachment_gates_only_the_affected_role(): + """Author attached, reviewer not: author work proceeds, review fails closed.""" + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=["gitea-author", "gitea-tools"], + required_namespaces=REQUIRED, + ) + store = _session_store_from(res) + assert mcp_namespace_health.attachment_gate_from_session("work_issue", store) == [] + assert mcp_namespace_health.attachment_gate_from_session("review_pr", store) + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + + +# --- AC4: no false healthy report without attachment proof ------------------ + + +def test_no_false_healthy_without_attachment_proof(): + """Connected alone never yields a healthy verdict.""" + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=REQUIRED, + attached_session_namespaces=None, + required_namespaces=REQUIRED, + ) + assert res["success"] is False + assert res["attachment_healthy"] is False + assert res["telemetry"]["attached_count"] == 0 + + +def test_attachment_gate_maps_each_role_namespace(): + assert mcp_namespace_health.required_namespace_for_attachment("review_pr") == "gitea-reviewer" + assert mcp_namespace_health.required_namespace_for_attachment("merge_pr") == "gitea-merger" + assert mcp_namespace_health.required_namespace_for_attachment("create_pr") == "gitea-author" + assert mcp_namespace_health.required_namespace_for_attachment("nope") is None -- 2.43.7 From 126d76ad2871f0782d5c6f40d53cc400557f0052 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Tue, 28 Jul 2026 16:46:37 -0400 Subject: [PATCH 3/5] docs(remote-mcp): restamp the commit the #956 anchors resolve at The anchors were re-derived against the #708 wiring commit; record that SHA so the fixture states the tree its line numbers were taken from. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/remote-mcp/threat-model-anchors.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/remote-mcp/threat-model-anchors.json b/docs/remote-mcp/threat-model-anchors.json index 04fe37f..24e9dd2 100644 --- a/docs/remote-mcp/threat-model-anchors.json +++ b/docs/remote-mcp/threat-model-anchors.json @@ -8,7 +8,7 @@ "document. #930's inventory had no such guard and its gitea_mcp_server.py", "anchors drifted between 7bf4f125 and aad5c8b4." ], - "generated_against_commit": "a143cd065ba06e1a2bdc5143a19ec156e53650ef", + "generated_against_commit": "e3fa3b263d4b8a04b189e111a345ac5c3a3fe1b6", "anchors": [ { "anchor": "gitea_mcp_server.py:24848", -- 2.43.7 From ca5f078d8a575ea3e2991771f8b4ea85e3dcaaa0 Mon Sep 17 00:00:00 2001 From: jcwalker3 Date: Tue, 28 Jul 2026 17:08:44 -0500 Subject: [PATCH 4/5] fix(mcp): separate not-connected from Connected-but-unattached (review 637 B2) A required namespace absent from the connected-service inventory was never counted as missing, because missing_namespaces was populated only while walking connected_servers. The session therefore reported attachment_healthy true, discovery_status namespaces_attached and error_type None while holding no attachment proof for that namespace, and the gate text told the operator "the host reports Connected" about a service nothing had reported Connected. assess_connected_namespace_attachment now classifies the two conditions apart. missing_namespaces keeps its meaning: required, Connected at the host, absent from the session tool surface -- the actual #708 defect, still typed mcp_connected_namespaces_missing. Required namespaces absent from the connected inventory land in the new not_connected_namespaces list, typed mcp_required_namespaces_not_connected, so a caller is never pointed at an attachment recovery for a service that never connected. attachment_healthy is false whenever any required namespace lacks attachment proof under either condition, error_types reports every condition present so neither hides the other, and namespace_conditions carries the per-namespace connected/attached verdict. Evidence that disagrees with itself -- attached in the session yet absent from the connected inventory -- is reported as contradictory and fails closed rather than being read as proof. attachment_gate_from_session states only what the recorded evidence supports: the Connected wording appears solely when connected is true, the not-connected wording when it is false, and a neutral refusal when connected status was never recorded. Review and merge stay fail-closed in all three cases. _record_live_namespace_attachment consumes the per-namespace verdicts instead of pinning the session-wide error_type onto every unattached namespace and instead of deriving health from a missing list that tracked only one of the two conditions. The two existing cases in test_issue_708_mcp_namespace_attachment.py declared no required_namespaces, so they inherited a default set containing a namespace their connected list omitted. Under the corrected rule that is a false-healthy assertion, so each now declares the required set it actually means. Closes #708 Co-Authored-By: Claude Opus 4.8 (1M context) --- gitea_mcp_server.py | 24 +- mcp_namespace_health.py | 197 +++++++++--- ...test_issue_708_mcp_namespace_attachment.py | 8 + ..._issue_708_not_connected_classification.py | 283 ++++++++++++++++++ 4 files changed, 473 insertions(+), 39 deletions(-) create mode 100644 tests/test_issue_708_not_connected_classification.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index bd01860..e82a7bd 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -6731,17 +6731,33 @@ def _record_live_namespace_attachment(assessment: dict | None) -> None: return missing = set(assessment.get("missing_namespaces") or []) error_type = assessment.get("error_type") + # Per-namespace verdicts when the assessment supplies them (#708 B2): a session-wide + # error_type must never be pinned onto a namespace whose own condition differs, and a + # namespace that was never connected must not be recorded healthy just because the + # session-wide "missing" list only tracks Connected-but-unattached namespaces. + conditions = assessment.get("namespace_conditions") + conditions = conditions if isinstance(conditions, dict) else {} for ns, state in proof.items(): ns_name = str(ns or "").strip() if not ns_name or not isinstance(state, dict): continue - attached = bool(state.get("attached")) + condition = conditions.get(ns_name) + condition = condition if isinstance(condition, dict) else {} + connected = bool(condition.get("connected", state.get("connected"))) + attached = bool(condition.get("attached", state.get("attached"))) + if condition: + healthy = bool(condition.get("attachment_healthy")) + ns_error = condition.get("condition") + else: + healthy = attached and connected and ns_name not in missing + ns_error = None if healthy else error_type _LIVE_NAMESPACE_ATTACHMENT[ns_name] = { "namespace": ns_name, - "connected": bool(state.get("connected")), + "connected": connected, "attached": attached, - "attachment_healthy": attached and ns_name not in missing, - "error_type": None if attached else error_type, + "attachment_healthy": healthy, + "condition": ns_error, + "error_type": ns_error, "discovery_status": assessment.get("discovery_status"), "reconnect_required": bool(assessment.get("reconnect_required")), "auto_recovered": bool(assessment.get("auto_recovered")), diff --git a/mcp_namespace_health.py b/mcp_namespace_health.py index 89999a5..e8e4910 100644 --- a/mcp_namespace_health.py +++ b/mcp_namespace_health.py @@ -53,6 +53,19 @@ EOF_PATTERNS = ( ERROR_CONNECTED_NAMESPACES_MISSING = "mcp_connected_namespaces_missing" +# Distinct from the condition above. ``mcp_connected_namespaces_missing`` is reserved for +# the actual #708 defect: the host *does* report the service Connected, yet the namespace +# never entered the active session tool surface. A required namespace that is absent from +# the connected-service inventory has no Connected claim behind it at all, so reporting it +# under the #708 condition would assert something the evidence does not support and would +# point an operator at the wrong recovery. +ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED = "mcp_required_namespaces_not_connected" + +DISCOVERY_STATUS_ATTACHED = "namespaces_attached" +DISCOVERY_STATUS_CONNECTED_MISSING = "connected_but_namespaces_missing" +DISCOVERY_STATUS_NOT_CONNECTED = "required_namespaces_not_connected" +DISCOVERY_STATUS_DISCONNECTED = "disconnected" + # Namespaces that must be *attached to the active session* for a mutation task (#708). # Connected-at-host is not attached-in-session; these are gated separately from the # #543 health map because a namespace can be healthy on probe yet absent from the @@ -115,27 +128,78 @@ def assess_connected_namespace_attachment( attached = set(str(ns).strip() for ns in (attached_session_namespaces or []) if str(ns).strip()) req = [str(r).strip() for r in (required_namespaces or DEFAULT_NAMESPACES) if str(r).strip()] + required = list(dict.fromkeys(req)) + connected_set = set(connected) + proof: dict[str, dict[str, bool]] = {} - missing: list[str] = [] - for s in connected: - is_attached = s in attached - proof[s] = {"connected": True, "attached": is_attached} - if not is_attached and s in req: - missing.append(s) - - for r in req: + proof[s] = {"connected": True, "attached": s in attached} + for r in required: if r not in proof: - proof[r] = {"connected": r in connected, "attached": r in attached} - if r in connected and r not in attached and r not in missing: - missing.append(r) + proof[r] = {"connected": r in connected_set, "attached": r in attached} - attachment_healthy = len(missing) == 0 and len(connected) > 0 - discovery_status = ( - "namespaces_attached" if attachment_healthy - else ("connected_but_namespaces_missing" if len(connected) > 0 else "disconnected") + # The genuine #708 condition: the host reports the service Connected and the namespace + # still never entered the active session tool surface. + missing = [r for r in required if r in connected_set and r not in attached] + # A separate condition: the service is required but absent from the connected-service + # inventory. Nothing here is Connected, so this must not borrow the #708 wording or its + # recovery — see ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED. + not_connected = [r for r in required if r not in connected_set] + # Evidence that disagrees with itself: reported attached to the session while absent + # from the connected inventory. Neither statement is proof, so it fails closed. + contradictory = [r for r in not_connected if r in attached] + + # No required namespace may lack attachment proof and still be called healthy. + attachment_healthy = ( + not missing + and not not_connected + and len(connected) > 0 + and len(required) > 0 ) + if attachment_healthy: + discovery_status = DISCOVERY_STATUS_ATTACHED + elif not connected: + discovery_status = DISCOVERY_STATUS_DISCONNECTED + elif not_connected: + discovery_status = DISCOVERY_STATUS_NOT_CONNECTED + else: + discovery_status = DISCOVERY_STATUS_CONNECTED_MISSING + + # Every condition actually present is reported; ``error_type`` names the primary one. + # Not-connected outranks Connected-but-unattached because a service that never + # connected cannot be recovered by attaching its namespace. + error_types: list[str] = [] + if not_connected: + error_types.append(ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED) + if missing: + error_types.append(ERROR_CONNECTED_NAMESPACES_MISSING) + if not attachment_healthy and not error_types: + error_types.append(ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED) + error_type = None if attachment_healthy else error_types[0] + + # Per-namespace verdict, so a mutation gate never has to infer one namespace's state + # from a whole-session summary. Each entry states only what its own evidence supports. + namespace_conditions: dict[str, dict[str, Any]] = {} + for ns_name, state in proof.items(): + ns_connected = bool(state["connected"]) + ns_attached = bool(state["attached"]) + if ns_connected and ns_attached: + condition = None + elif ns_connected: + condition = ERROR_CONNECTED_NAMESPACES_MISSING + else: + condition = ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED + namespace_conditions[ns_name] = { + "namespace": ns_name, + "required": ns_name in required, + "connected": ns_connected, + "attached": ns_attached, + "attachment_healthy": ns_connected and ns_attached, + "condition": condition, + "contradictory_evidence": ns_attached and not ns_connected, + } + # Startup-ordering race: a namespace that finished connecting *after* the session # tool snapshot was taken cannot be in that snapshot, however healthy it looks now. connected_at = { @@ -156,6 +220,9 @@ def assess_connected_namespace_attachment( reasons: list[str] = [] remediation: list[str] = [] + if not connected: + reasons.append("No MCP servers reported Connected.") + remediation.append("Start or reconnect Gitea MCP servers in client config.") if missing: reasons.append( f"MCP server(s) {missing} report Connected at host/CLI layer but tool namespaces " @@ -167,11 +234,28 @@ def assess_connected_namespace_attachment( "(gitea_whoami -> gitea_resolve_task_capability -> task). " "Do not use direct imports, CLI API mutations, profile hopping, or session file overrides." ) - elif not connected: - reasons.append("No MCP servers reported Connected.") - remediation.append("Start or reconnect Gitea MCP servers in client config.") - else: - reasons.append("All connected MCP server namespaces are attached to the active session.") + if not_connected: + reasons.append( + f"required MCP namespace(s) {not_connected} are absent from the connected-service " + "inventory: nothing reports them Connected, so there is no attachment to claim and " + "the Connected-but-unattached condition does not apply to them (#708)." + ) + remediation.append( + f"Connect the required MCP server(s) {not_connected} through the client, then " + "reconnect the IDE/client MCP session so their namespaces attach " + f"(sanctioned path: {SANCTIONED_ATTACH_RECOVERY_TOOL}), and re-run full preflight. " + "Do not use direct imports, CLI API mutations, profile hopping, or session file overrides." + ) + if contradictory: + reasons.append( + f"contradictory evidence for namespace(s) {contradictory}: reported attached to the " + "active session while absent from the connected-service inventory; neither statement " + "is proof, so attachment is treated as unproven (fail closed, #708)." + ) + if attachment_healthy: + reasons.append( + "All required MCP server namespaces are connected and attached to the active session." + ) if startup_ordering_race: reasons.append( @@ -193,15 +277,29 @@ def assess_connected_namespace_attachment( "connected_servers": connected, "attached_session_namespaces": list(attached), "missing_namespaces": missing, + "not_connected_namespaces": not_connected, + "contradictory_namespaces": contradictory, "proof_of_connected_vs_attached": proof, - "error_type": None if attachment_healthy else ERROR_CONNECTED_NAMESPACES_MISSING, + "namespace_conditions": namespace_conditions, + "error_type": error_type, + "error_types": error_types, "reasons": reasons, "remediation": remediation, "exact_next_action": ( - "Reconnect the IDE/client MCP session so tool namespaces attach to the active session. " - "Do not use direct imports, CLI API mutations, profile hopping, or session-state overrides." - if not attachment_healthy - else "None; session tool namespaces attached." + "None; session tool namespaces attached." + if attachment_healthy + else ( + f"Connect the required MCP server(s) {not_connected} through the client, then " + "reconnect the IDE/client MCP session so their tool namespaces attach to the " + "active session. Do not use direct imports, CLI API mutations, profile hopping, " + "or session-state overrides." + if not_connected + else ( + "Reconnect the IDE/client MCP session so tool namespaces attach to the active " + "session. Do not use direct imports, CLI API mutations, profile hopping, or " + "session-state overrides." + ) + ) ), "unsafe_fallback_policy": UNSAFE_FALLBACK_WARNING, "sanctioned_recovery_tool": SANCTIONED_ATTACH_RECOVERY_TOOL, @@ -215,8 +313,11 @@ def assess_connected_namespace_attachment( "telemetry": { "connected_count": len(connected), "attached_count": len(attached), - "required_count": len(req), + "required_count": len(required), "missing_count": len(missing), + "not_connected_count": len(not_connected), + "contradictory_count": len(contradictory), + "required_attached_count": sum(1 for r in required if r in attached), "discovery_status": discovery_status, "discovery_cache_hit": ( None if discovery_cache_hit is None else bool(discovery_cache_hit) @@ -230,7 +331,8 @@ def assess_connected_namespace_attachment( "auto_attach_attempted": bool(auto_attach_attempted), "auto_recovered": auto_recovered, "startup_ordering_race": startup_ordering_race, - "error_type": None if attachment_healthy else ERROR_CONNECTED_NAMESPACES_MISSING, + "error_type": error_type, + "error_types": error_types, }, } @@ -261,14 +363,39 @@ def attachment_gate_from_session( return [] if entry.get("attached") and entry.get("attachment_healthy"): return [] - detail = entry.get("error_type") or ERROR_CONNECTED_NAMESPACES_MISSING - return [ - f"live MCP namespace '{ns}' is recorded {detail}: the host reports Connected but the " - f"namespace is not attached to the active session tool surface; reconnect the " - f"IDE/client MCP session and re-run preflight before {(task or 'mutation')} " - "(fail closed, #708)", - UNSAFE_FALLBACK_WARNING, - ] + + what = task or "mutation" + connected = entry.get("connected") + detail = entry.get("condition") or entry.get("error_type") + if connected is True: + # Only here has anything actually reported the service Connected, so only here may + # the block say so. + detail = detail or ERROR_CONNECTED_NAMESPACES_MISSING + blocked = ( + f"live MCP namespace '{ns}' is recorded {detail}: the host reports Connected but the " + f"namespace is not attached to the active session tool surface; reconnect the " + f"IDE/client MCP session and re-run preflight before {what} " + "(fail closed, #708)" + ) + elif connected is False: + detail = ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED + blocked = ( + f"live MCP namespace '{ns}' is recorded {detail}: it is absent from the " + f"connected-service inventory, so it is neither connected nor attached and no " + f"Connected status is claimed for it; connect the required MCP server, then " + f"reconnect the IDE/client MCP session and re-run preflight before {what} " + "(fail closed, #708)" + ) + else: + # Connected status was never recorded. Refuse without asserting either condition. + detail = detail or ERROR_CONNECTED_NAMESPACES_MISSING + blocked = ( + f"live MCP namespace '{ns}' is recorded {detail} with no connected-status evidence, " + f"so attachment to the active session tool surface is unproven; reconnect the " + f"IDE/client MCP session and re-run preflight before {what} " + "(fail closed, #708)" + ) + return [blocked, UNSAFE_FALLBACK_WARNING] diff --git a/tests/test_issue_708_mcp_namespace_attachment.py b/tests/test_issue_708_mcp_namespace_attachment.py index 3d26b3a..e5a8fcd 100644 --- a/tests/test_issue_708_mcp_namespace_attachment.py +++ b/tests/test_issue_708_mcp_namespace_attachment.py @@ -4,11 +4,16 @@ import mcp_namespace_health def test_assess_connected_namespace_attachment_success(): + # required_namespaces is declared explicitly: these three are the namespaces this case + # is about. Relying on the default (which also requires gitea-tools) would ask for a + # healthy verdict covering a required namespace that was never connected — exactly the + # false-healthy classification these tests now forbid. connected = ["gitea-author", "gitea-reviewer", "gitea-merger"] attached = ["gitea-author", "gitea-reviewer", "gitea-merger"] res = mcp_namespace_health.assess_connected_namespace_attachment( connected_servers=connected, attached_session_namespaces=attached, + required_namespaces=connected, ) assert res["success"] is True assert res["attachment_healthy"] is True @@ -19,11 +24,14 @@ def test_assess_connected_namespace_attachment_success(): def test_assess_connected_namespace_attachment_missing(): + # Every required namespace here *is* connected, so the only condition present is the + # #708 one: Connected at the host, absent from the session tool surface. connected = ["gitea-author", "gitea-reviewer", "gitea-merger"] attached = ["gitea-author"] res = mcp_namespace_health.assess_connected_namespace_attachment( connected_servers=connected, attached_session_namespaces=attached, + required_namespaces=connected, ) assert res["success"] is False assert res["attachment_healthy"] is False diff --git a/tests/test_issue_708_not_connected_classification.py b/tests/test_issue_708_not_connected_classification.py new file mode 100644 index 0000000..02bcc39 --- /dev/null +++ b/tests/test_issue_708_not_connected_classification.py @@ -0,0 +1,283 @@ +"""Regression tests for Issue #708 B2: not-connected is not Connected-but-unattached. + +The first #708 slice counted a namespace as ``missing`` only when it appeared in +``connected_servers``. A *required* namespace absent from that inventory was therefore +never counted at all, so the session reported ``attachment_healthy: true`` with +``error_type: None`` while holding no attachment proof for it — and the gate text told the +operator "the host reports Connected" about a service nothing had reported Connected. + +These tests pin the corrected distinction: + +* required and not connected never yields a healthy verdict, +* it is typed ``mcp_required_namespaces_not_connected``, never the #708 condition, +* ``mcp_connected_namespaces_missing`` stays reserved for genuinely Connected namespaces, +* both categories still fail review and merge closed, with their own reason, +* per-namespace ``connected``/``attached`` evidence is reported accurately, +* one session's evidence cannot clear another session's block. +""" + +import mcp_namespace_health + + +ROLES = ["gitea-author", "gitea-reviewer", "gitea-merger", "gitea-tools"] + +NOT_CONNECTED = mcp_namespace_health.ERROR_REQUIRED_NAMESPACES_NOT_CONNECTED +CONNECTED_MISSING = mcp_namespace_health.ERROR_CONNECTED_NAMESPACES_MISSING + + +def _assess(connected, attached, required): + return mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=connected, + attached_session_namespaces=attached, + required_namespaces=required, + ) + + +def _store(assessment): + """The recorder contract: per-namespace verdicts feed the gate.""" + return dict(assessment["namespace_conditions"]) + + +# --- the four evidence combinations ---------------------------------------- + + +def test_connected_and_attached_is_healthy(): + res = _assess(ROLES, ROLES, ROLES) + assert res["attachment_healthy"] is True + assert res["error_type"] is None + assert res["missing_namespaces"] == [] + assert res["not_connected_namespaces"] == [] + assert res["discovery_status"] == "namespaces_attached" + + +def test_connected_but_unattached_keeps_the_708_condition(): + res = _assess(ROLES, ["gitea-author"], ROLES) + assert res["attachment_healthy"] is False + assert res["error_type"] == CONNECTED_MISSING + assert res["not_connected_namespaces"] == [] + assert sorted(res["missing_namespaces"]) == [ + "gitea-merger", + "gitea-reviewer", + "gitea-tools", + ] + assert res["discovery_status"] == "connected_but_namespaces_missing" + + +def test_required_but_not_connected_is_never_healthy(): + """The reviewer's exact reproduction from review 637.""" + res = _assess( + ["gitea-reviewer"], ["gitea-reviewer"], ["gitea-reviewer", "gitea-merger"] + ) + assert res["attachment_healthy"] is False + assert res["success"] is False + assert res["error_type"] is not None + assert res["telemetry"]["not_connected_count"] == 1 + + +def test_required_but_not_connected_is_typed_distinctly(): + res = _assess( + ["gitea-reviewer"], ["gitea-reviewer"], ["gitea-reviewer", "gitea-merger"] + ) + assert res["error_type"] == NOT_CONNECTED + assert res["discovery_status"] == "required_namespaces_not_connected" + assert res["not_connected_namespaces"] == ["gitea-merger"] + # Not collapsed into the #708 condition, nor into config drift (#672) or + # transport-closed (#584). + assert CONNECTED_MISSING not in res["error_types"] + assert res["missing_namespaces"] == [] + assert res["error_type"] not in {"mcp_config_drift", "transport_closed"} + + +def test_not_connected_reason_makes_no_connected_claim(): + res = _assess( + ["gitea-reviewer"], ["gitea-reviewer"], ["gitea-reviewer", "gitea-merger"] + ) + about_merger = [r for r in res["reasons"] if "gitea-merger" in r] + assert about_merger, "the not-connected namespace must be named in the reasons" + for reason in about_merger: + assert "report Connected at host/CLI layer" not in reason + assert "gitea-merger" in res["exact_next_action"] + + +def test_neither_connected_nor_attached_reports_disconnected(): + res = _assess([], [], ROLES) + assert res["attachment_healthy"] is False + assert res["discovery_status"] == "disconnected" + assert res["error_type"] == NOT_CONNECTED + assert sorted(res["not_connected_namespaces"]) == sorted(ROLES) + assert res["missing_namespaces"] == [] + + +# --- mixed required namespaces --------------------------------------------- + + +def test_mixed_connected_attached_and_not_connected(): + res = _assess( + ["gitea-author"], ["gitea-author"], ["gitea-author", "gitea-merger"] + ) + assert res["attachment_healthy"] is False + assert res["not_connected_namespaces"] == ["gitea-merger"] + assert res["missing_namespaces"] == [] + conditions = res["namespace_conditions"] + assert conditions["gitea-author"]["attachment_healthy"] is True + assert conditions["gitea-author"]["condition"] is None + assert conditions["gitea-merger"]["attachment_healthy"] is False + assert conditions["gitea-merger"]["condition"] == NOT_CONNECTED + + +def test_both_conditions_present_are_both_reported(): + """One namespace Connected-but-unattached, another never connected.""" + res = _assess( + ["gitea-author", "gitea-reviewer"], + ["gitea-author"], + ["gitea-author", "gitea-reviewer", "gitea-merger"], + ) + assert res["missing_namespaces"] == ["gitea-reviewer"] + assert res["not_connected_namespaces"] == ["gitea-merger"] + # Neither condition is hidden by the other; error_type names the primary one. + assert sorted(res["error_types"]) == sorted([NOT_CONNECTED, CONNECTED_MISSING]) + assert res["error_type"] == NOT_CONNECTED + + +# --- unknown, partial, malformed, contradictory evidence -------------------- + + +def test_unknown_attachment_evidence_is_not_healthy(): + """No session tool surface reported at all is unproven, not proven good.""" + res = _assess(ROLES, None, ROLES) + assert res["attachment_healthy"] is False + assert res["telemetry"]["attached_count"] == 0 + assert res["error_type"] == CONNECTED_MISSING + + +def test_partial_evidence_gates_only_the_unproven_roles(): + res = _assess(ROLES, ["gitea-author", "gitea-tools"], ROLES) + store = _store(res) + assert mcp_namespace_health.attachment_gate_from_session("work_issue", store) == [] + assert mcp_namespace_health.attachment_gate_from_session("review_pr", store) + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + + +def test_malformed_namespace_entries_are_discarded_not_trusted(): + """Blank and whitespace-only names must not become namespaces or proof.""" + res = mcp_namespace_health.assess_connected_namespace_attachment( + connected_servers=["gitea-author", "", " "], + attached_session_namespaces=["gitea-author", ""], + required_namespaces=["gitea-author", " "], + ) + assert "" not in res["proof_of_connected_vs_attached"] + assert " " not in res["proof_of_connected_vs_attached"] + assert res["attachment_healthy"] is True + assert res["telemetry"]["required_count"] == 1 + + +def test_contradictory_attached_without_connected_fails_closed(): + """Attached in the session yet absent from the connected inventory.""" + res = _assess(["gitea-author"], ["gitea-author", "gitea-merger"], ROLES) + assert res["attachment_healthy"] is False + assert "gitea-merger" in res["contradictory_namespaces"] + assert "gitea-merger" in res["not_connected_namespaces"] + assert res["namespace_conditions"]["gitea-merger"]["contradictory_evidence"] is True + assert res["namespace_conditions"]["gitea-merger"]["attachment_healthy"] is False + assert any("contradictory evidence" in r for r in res["reasons"]) + assert res["telemetry"]["contradictory_count"] >= 1 + + +def test_duplicate_required_entries_are_counted_once(): + res = _assess(["gitea-author"], ["gitea-author"], ["gitea-author", "gitea-author"]) + assert res["telemetry"]["required_count"] == 1 + assert res["attachment_healthy"] is True + + +# --- review and merge behaviour for both failure categories ----------------- + + +def test_merge_fails_closed_when_required_namespace_never_connected(): + store = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert reasons + assert NOT_CONNECTED in reasons[0] + assert "fail closed, #708" in reasons[0] + + +def test_review_fails_closed_when_required_namespace_never_connected(): + store = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("review_pr", store) + assert reasons + assert NOT_CONNECTED in reasons[0] + assert "fail closed, #708" in reasons[0] + + +def test_not_connected_block_never_claims_the_host_reports_connected(): + store = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert "the host reports Connected" not in reasons[0] + assert "absent from the connected-service inventory" in reasons[0] + + +def test_connected_but_unattached_block_still_says_connected(): + store = _store(_assess(ROLES, ["gitea-author"], ROLES)) + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert CONNECTED_MISSING in reasons[0] + assert "the host reports Connected" in reasons[0] + + +def test_both_categories_offer_only_the_sanctioned_recovery(): + for store in ( + _store(_assess(ROLES, [], ROLES)), + _store(_assess(["gitea-author"], ["gitea-author"], ROLES)), + ): + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + joined = " ".join(reasons) + assert "reconnect" in joined.lower() + assert "Workflow Safety Hard Stop (#708)" in joined + for forbidden in ("pkill", "chmod", "curl ", ".env", "sys.path"): + assert forbidden not in reasons[0] + + +def test_entry_without_connected_evidence_blocks_without_asserting_either(): + """A legacy/partial store entry must fail closed and claim nothing it cannot prove.""" + store = {"gitea-merger": {"namespace": "gitea-merger", "attached": False}} + reasons = mcp_namespace_health.attachment_gate_from_session("merge_pr", store) + assert reasons + assert "no connected-status evidence" in reasons[0] + assert "the host reports Connected" not in reasons[0] + assert "fail closed, #708" in reasons[0] + + +# --- session isolation ------------------------------------------------------ + + +def test_one_session_evidence_does_not_clear_another_session_block(): + """Attachment evidence is per-session state; it must not travel between sessions.""" + blocked_session = _store(_assess(["gitea-author"], ["gitea-author"], ROLES)) + healthy_session = _store(_assess(ROLES, ROLES, ROLES)) + + assert ( + mcp_namespace_health.attachment_gate_from_session("merge_pr", healthy_session) + == [] + ) + # The healthy session's verdict is not consulted for the blocked session. + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", blocked_session) + # And the blocked session's store is unchanged by the healthy one existing. + assert blocked_session["gitea-merger"]["attachment_healthy"] is False + + +def test_gate_reads_only_the_store_it_is_given(): + healthy_session = _store(_assess(ROLES, ROLES, ROLES)) + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", {}) == [] + assert mcp_namespace_health.attachment_gate_from_session("merge_pr", None) == [] + assert ( + mcp_namespace_health.attachment_gate_from_session("merge_pr", healthy_session) + == [] + ) + + +# --- telemetry stays secret-free ------------------------------------------- + + +def test_not_connected_telemetry_leaks_no_secrets(): + res = _assess(["gitea-author"], ["gitea-author"], ROLES) + blob = repr(res["telemetry"]).lower() + for leak in ("token", "authorization", "password", "secret", "/users/", "http"): + assert leak not in blob -- 2.43.7 From 58bd8521880fe464d5d816b11d1001ffaf7cbdfa Mon Sep 17 00:00:00 2001 From: jcwalker3 Date: Tue, 28 Jul 2026 17:25:24 -0500 Subject: [PATCH 5/5] docs(remote-mcp): restamp the #956 threat model onto the commit it resolves at (review 637 B1) Commit 126d76a set docs/remote-mcp/threat-model-anchors.json generated_against_commit to e3fa3b26 and left docs/remote-mcp/threat-model.md citing a143cd06, so the document and the fixture named different source revisions. tests/test_issue_956_threat_model.py asserts the fixture value appears in the document, and that assertion failed from 126d76a onward. The mismatch was not cosmetic. Resolving all 58 anchors against each candidate revision shows 0 unresolved at e3fa3b26 and 21 unresolved at a143cd06, so the document's own claim about where its file:line citations resolve was false. The fixture held the accurate value and the document held the stale one. The B2 fix in ca5f078d inserted a net 16 lines into gitea_mcp_server.py at a single point, shifting 21 anchors below it. All 21 are re-derived here by that one uniform offset and each was verified against its recorded expect substring rather than assumed, so no anchor is guessed and none became ambiguous. Both artifacts now name ca5f078d, the commit those anchors were taken at, and the document's inline citations are shifted to match. The boundary table's claim about what the code enforces is restamped onto the same revision, because it described that same source tree. Verified: 58/58 anchors resolve at the declared generation commit, 58/58 at this head, no two anchors share a file:line location, fixture and document metadata agree, and tests/test_issue_956_threat_model.py passes 17/17. Refs #708 Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/remote-mcp/threat-model-anchors.json | 44 +++++++++--------- docs/remote-mcp/threat-model.md | 54 ++++++++++++----------- 2 files changed, 50 insertions(+), 48 deletions(-) diff --git a/docs/remote-mcp/threat-model-anchors.json b/docs/remote-mcp/threat-model-anchors.json index 24e9dd2..33e81ef 100644 --- a/docs/remote-mcp/threat-model-anchors.json +++ b/docs/remote-mcp/threat-model-anchors.json @@ -8,10 +8,10 @@ "document. #930's inventory had no such guard and its gitea_mcp_server.py", "anchors drifted between 7bf4f125 and aad5c8b4." ], - "generated_against_commit": "e3fa3b263d4b8a04b189e111a345ac5c3a3fe1b6", + "generated_against_commit": "ca5f078d8a575ea3e2991771f8b4ea85e3dcaaa0", "anchors": [ { - "anchor": "gitea_mcp_server.py:24848", + "anchor": "gitea_mcp_server.py:24864", "expect": "mcp_daemon_guard.bind_native_mcp_transport()" }, { @@ -27,11 +27,11 @@ "expect": "def assess_transport_for_auth_mint" }, { - "anchor": "gitea_mcp_server.py:9175", + "anchor": "gitea_mcp_server.py:9191", "expect": "assess_transport_for_auth_mint()" }, { - "anchor": "gitea_mcp_server.py:9424", + "anchor": "gitea_mcp_server.py:9440", "expect": "assess_transport_for_auth_mint()" }, { @@ -39,31 +39,31 @@ "expect": "The transport is selected by deployment configuration" }, { - "anchor": "gitea_mcp_server.py:15465", + "anchor": "gitea_mcp_server.py:15481", "expect": "def _is_client_managed_process" }, { - "anchor": "gitea_mcp_server.py:15495", + "anchor": "gitea_mcp_server.py:15511", "expect": "def _provenance_mutation_block" }, { - "anchor": "gitea_mcp_server.py:15503", + "anchor": "gitea_mcp_server.py:15519", "expect": "unsupported_manual_launch" }, { - "anchor": "gitea_mcp_server.py:19054", + "anchor": "gitea_mcp_server.py:19070", "expect": "server_provenance" }, { - "anchor": "gitea_mcp_server.py:21565", + "anchor": "gitea_mcp_server.py:21581", "expect": "def _check_mcp_runtimes_diagnostics" }, { - "anchor": "gitea_mcp_server.py:21585", + "anchor": "gitea_mcp_server.py:21601", "expect": "\"ps\", \"-o\", \"pid,lstart,command\"" }, { - "anchor": "gitea_mcp_server.py:21629", + "anchor": "gitea_mcp_server.py:21645", "expect": "\"ps\", \"eww\"" }, { @@ -107,19 +107,19 @@ "expect": "def assert_keychain_access_allowed" }, { - "anchor": "gitea_mcp_server.py:19311", + "anchor": "gitea_mcp_server.py:19327", "expect": "def gitea_list_profiles" }, { - "anchor": "gitea_mcp_server.py:19362", + "anchor": "gitea_mcp_server.py:19378", "expect": "gitea_config.resolve_token(p)" }, { - "anchor": "gitea_mcp_server.py:19675", + "anchor": "gitea_mcp_server.py:19691", "expect": "def gitea_audit_config" }, { - "anchor": "gitea_mcp_server.py:19697", + "anchor": "gitea_mcp_server.py:19713", "expect": "service_summaries(config)" }, { @@ -135,19 +135,19 @@ "expect": "_keychain_token(auth.get(\"id\"))" }, { - "anchor": "gitea_mcp_server.py:17760", + "anchor": "gitea_mcp_server.py:17776", "expect": "\"jenkins-mcp\"" }, { - "anchor": "gitea_mcp_server.py:17766", + "anchor": "gitea_mcp_server.py:17782", "expect": "external-mcp" }, { - "anchor": "gitea_mcp_server.py:17787", + "anchor": "gitea_mcp_server.py:17803", "expect": "\"glitchtip-mcp\"" }, { - "anchor": "gitea_mcp_server.py:17792", + "anchor": "gitea_mcp_server.py:17808", "expect": "external-mcp" }, { @@ -183,7 +183,7 @@ "expect": "mutation_safe" }, { - "anchor": "gitea_mcp_server.py:19155", + "anchor": "gitea_mcp_server.py:19171", "expect": "def gitea_assess_master_parity" }, { @@ -199,7 +199,7 @@ "expect": "/tmp/gitea_issue_lock.json" }, { - "anchor": "gitea_mcp_server.py:10940", + "anchor": "gitea_mcp_server.py:10956", "expect": "def gitea_bootstrap_author_issue_worktree" }, { @@ -239,7 +239,7 @@ "expect": "os.getpid()" }, { - "anchor": "gitea_mcp_server.py:12854", + "anchor": "gitea_mcp_server.py:12870", "expect": "owner_pid_alive" } ] diff --git a/docs/remote-mcp/threat-model.md b/docs/remote-mcp/threat-model.md index 9518185..ddbb7a6 100644 --- a/docs/remote-mcp/threat-model.md +++ b/docs/remote-mcp/threat-model.md @@ -5,9 +5,11 @@ What the adversary is, what each boundary protects, and which services may share - **Issue:** #956 (Remote-MCP threat model), child of epic #929, cross-linked to #955. - **Depends on:** #930 (closed) — `docs/remote-mcp/coupling-inventory.md`. - **Blocks:** #932, #933, #934, #938. -- **Generated against commit:** `a143cd065ba06e1a2bdc5143a19ec156e53650ef` (#931's transport - bind seam). Originally generated against `aad5c8b42361d380a8eeb07b94b90815e594c2c5` - (`master`) and re-anchored when #931 shifted the cited lines. +- **Generated against commit:** `ca5f078d8a575ea3e2991771f8b4ea85e3dcaaa0` (#708's + namespace-attachment gate). Originally generated against + `aad5c8b42361d380a8eeb07b94b90815e594c2c5` (`master`), re-anchored at + `a143cd065ba06e1a2bdc5143a19ec156e53650ef` when #931's transport bind seam shifted the + cited lines, and re-anchored again when #708 shifted them further. - **Scope:** documentation only. This child changes no server behavior. It adds one document, one anchor fixture, and the test that enforces them. @@ -29,7 +31,7 @@ document cites an anchor the fixture does not cover. This guard exists because #930 did not have one. Its inventory was generated at `7bf4f125`; by `aad5c8b4` its `gitea_mcp_server.py` anchors had drifted — the transport -bind it cited at line 23750 now lives at `gitea_mcp_server.py:24848`, and its +bind it cited at line 23750 now lives at `gitea_mcp_server.py:24864`, and its client-managed provenance anchor at 14588 now lands in an unrelated function. Nothing failed, because nothing checked. Anchors into a ~24,700-line module rot silently, and a security document that cannot prove its own citations is worse than none, because it is @@ -73,19 +75,19 @@ authenticate the *caller*, not the *intent*. ## 3. Trust boundaries "Crossing requires today" is what the code actually enforces at -`a143cd065ba06e1a2bdc5143a19ec156e53650ef`, not what the design intends. +`ca5f078d8a575ea3e2991771f8b4ea85e3dcaaa0`, not what the design intends. | ID | Boundary | Protects | Crossing requires today | Crossing must require remotely | | -- | -------- | -------- | ----------------------- | ------------------------------ | -| B1 | LLM client ↔ MCP server session | A1, A3, A10 — that a mutating session was established through the sanctioned client path | A single configured bind (`gitea_mcp_server.py:24848`) validated against one closed allowlist (`mcp_daemon_guard.py:49`, `mcp_daemon_guard.py:195`) — since #931 the identifier comes from deployment configuration and defaults to the local transport, so the boundary no longer rests on a literal, but it still rests on the *bind* rather than on an authenticated caller; client-managed provenance (`gitea_mcp_server.py:15465`) or a refusal (`gitea_mcp_server.py:15503`); production transport before recovery-authorization mint (`irrecoverable_provenance.py:497`, consumed at `gitea_mcp_server.py:9175` and `gitea_mcp_server.py:9424`) | An authenticated handshake issuing a server-side session identity bound to a principal, with the transport recorded in provenance. The physical proof (a pipe) must become a cryptographic one. | +| B1 | LLM client ↔ MCP server session | A1, A3, A10 — that a mutating session was established through the sanctioned client path | A single configured bind (`gitea_mcp_server.py:24864`) validated against one closed allowlist (`mcp_daemon_guard.py:49`, `mcp_daemon_guard.py:195`) — since #931 the identifier comes from deployment configuration and defaults to the local transport, so the boundary no longer rests on a literal, but it still rests on the *bind* rather than on an authenticated caller; client-managed provenance (`gitea_mcp_server.py:15481`) or a refusal (`gitea_mcp_server.py:15519`); production transport before recovery-authorization mint (`irrecoverable_provenance.py:497`, consumed at `gitea_mcp_server.py:9191` and `gitea_mcp_server.py:9440`) | An authenticated handshake issuing a server-side session identity bound to a principal, with the transport recorded in provenance. The physical proof (a pipe) must become a cryptographic one. | | B2 | Role ↔ role | A9 — that author, reviewer, merger, and reconciler are distinct authorities | **The process boundary only.** The role is a property of the process, read once from `GITEA_MCP_PROFILE` (`gitea_config.py:54`). A caller gets author permissions by connecting to the author process. Review and merge are the operations singled out for extra care (`gitea_config.py:97`) | A per-request principal, so the role follows from the credential presented and cannot be selected by reaching a different endpoint. | | B3 | MCP server ↔ credential store | A3, A8 — that only sanctioned code turns a profile into a token | `_keychain_token` shelling out to the login keychain (`gitea_config.py:956`), dispatched by `resolve_token` (`gitea_config.py:974`) with the reference type built at `gitea_config.py:1015`, gated by `assert_keychain_access_allowed` (`mcp_daemon_guard.py:583`). Inline secrets are rejected at config load (`gitea_config.py:294`) | A credential provider keyed by the *request* principal, returning only that principal's credential, with the source recorded and the value never returned. | | B4 | MCP server ↔ Gitea | A1, A2 — that only authorized calls reach the forge | A bearer token over TLS. Server-side, nothing distinguishes one role's token from another beyond the account it belongs to | Unchanged at the forge; the endpoint in front of it must refuse unauthenticated and plaintext connections before tool dispatch. | -| B5 | MCP server ↔ caller's filesystem | A7 — that a tool acts on the *caller's* disk or refuses | Nothing. The server's disk *is* the caller's disk. Worktree bootstrap writes directly (`gitea_mcp_server.py:10940`); the active workspace is process-global (`gitea_mcp_server.py:193`, `gitea_mcp_server.py:194`) | An explicit per-tool classification, enforced at dispatch, refusing filesystem tools over a transport that cannot reach the caller's disk. A green verdict about the wrong disk is the failure to prevent. | -| B6 | MCP server ↔ coordination state | A6, A9 — mutual exclusion | Local files and a local SQLite database, with liveness judged from the local process table (`issue_lock_store.py:98`), keyed on paths under one user's home (`issue_lock_store.py:26`, `mcp_session_state.py:27`, `control_plane_db.py:47`) and on `os.getpid()` (`control_plane_db.py:1145`, `gitea_mcp_server.py:12854`). A legacy global slot still exists at `gitea_mcp_server.py:2351`, and the session-pointer file is named per PID (`issue_lock_store.py:83`) | One authority per ownership question, with liveness from session identity and expiry, and atomic acquire, renew, and release across hosts. | +| B5 | MCP server ↔ caller's filesystem | A7 — that a tool acts on the *caller's* disk or refuses | Nothing. The server's disk *is* the caller's disk. Worktree bootstrap writes directly (`gitea_mcp_server.py:10956`); the active workspace is process-global (`gitea_mcp_server.py:193`, `gitea_mcp_server.py:194`) | An explicit per-tool classification, enforced at dispatch, refusing filesystem tools over a transport that cannot reach the caller's disk. A green verdict about the wrong disk is the failure to prevent. | +| B6 | MCP server ↔ coordination state | A6, A9 — mutual exclusion | Local files and a local SQLite database, with liveness judged from the local process table (`issue_lock_store.py:98`), keyed on paths under one user's home (`issue_lock_store.py:26`, `mcp_session_state.py:27`, `control_plane_db.py:47`) and on `os.getpid()` (`control_plane_db.py:1145`, `gitea_mcp_server.py:12870`). A legacy global slot still exists at `gitea_mcp_server.py:2351`, and the session-pointer file is named per PID (`issue_lock_store.py:83`) | One authority per ownership question, with liveness from session identity and expiry, and atomic acquire, renew, and release across hosts. | | B7 | Gitea integration ↔ unrelated integrations | A4, A5 — that a Gitea compromise is not a CI and observability compromise | **Nothing.** See §5. The Gitea server reads Jenkins and GlitchTip secrets (`gitea_config.py:851`, reached from `gitea_config.py:837`) and holds the Sentry token (`sentry_incident_bridge.py:190`) | A hard process boundary. This is the boundary #956 exists to create. | | B8 | Tenant ↔ tenant (`prgs` / `mdcps` / `local-lab`) | A2 — that one organization's compromise is not another's | Convention. One configuration declares all three contexts; `resolve_service` fails closed on a *disabled* context (`gitea_config.py:704`) but the credentials of enabled ones remain reachable in-process. A per-profile repository scope exists (`gitea_config.py:499`) | Separate deployments, or at minimum per-tenant credential scopes with no process able to resolve both. | -| B9 | Deployed code ↔ merged policy | A1, A10 — that the running server enforces the rules that were actually merged | Comparing this process's startup commit against this disk (`master_parity_gate.py:168`), conjoined into a single verdict (`master_parity_gate.py:255`) published by `gitea_mcp_server.py:19155` | Freshness defined against the deployed build identity, with an explicit fail-closed verdict when undeterminable. | +| B9 | Deployed code ↔ merged policy | A1, A10 — that the running server enforces the rules that were actually merged | Comparing this process's startup commit against this disk (`master_parity_gate.py:168`), conjoined into a single verdict (`master_parity_gate.py:255`) published by `gitea_mcp_server.py:19171` | Freshness defined against the deployed build identity, with an explicit fail-closed verdict when undeterminable. | ### What no boundary constrains @@ -130,10 +132,10 @@ Two flows deserve attention because neither is obvious from the code: 1. **The keychain flow fans out.** B3 is drawn once but resolves credentials for *every* configured profile and service, not only the active one. `gitea_list_profiles` - (`gitea_mcp_server.py:19311`) reports each profile's credential status by calling - `resolve_token` on it (`gitea_mcp_server.py:19362`), and `gitea_audit_config` - (`gitea_mcp_server.py:19675`) reports service credential status through - `service_summaries` (`gitea_mcp_server.py:19697`). + (`gitea_mcp_server.py:19327`) reports each profile's credential status by calling + `resolve_token` on it (`gitea_mcp_server.py:19378`), and `gitea_audit_config` + (`gitea_mcp_server.py:19691`) reports service credential status through + `service_summaries` (`gitea_mcp_server.py:19713`). 2. **The return path is a flow too.** Content read from Gitea travels back into the model and is treated as instruction. This is the ADV2 edge, and it is the only edge in the diagram with no authentication on it, because it is not a request. @@ -178,16 +180,16 @@ the credential, and an attacker holding the token does not call our tools. **Finding 3 — Any one role process can resolve every other role's credential.** This is not inferred; it is demonstrated by tool output. `gitea_list_profiles` -(`gitea_mcp_server.py:19311`) called from the **author** session reports +(`gitea_mcp_server.py:19327`) called from the **author** session reports `identity_status: "credentials present"` for `prgs-merger`, `prgs-reviewer`, `prgs-reconciler`, and every `mdcps` profile, because it calls `resolve_token` on each one -(`gitea_mcp_server.py:19362`). The author process does not merely *have access to* the +(`gitea_mcp_server.py:19378`). The author process does not merely *have access to* the merger's credential — it reads it to answer a status query. B2 is not a credential boundary in either direction. **Finding 4 — The Gitea server reads CI and observability secrets.** `gitea_audit_config` -(`gitea_mcp_server.py:19675`) reports `MDCPS Jenkins: enabled, read-only, authenticated`. -That word `authenticated` is produced by `service_summaries` (`gitea_mcp_server.py:19697`, +(`gitea_mcp_server.py:19691`) reports `MDCPS Jenkins: enabled, read-only, authenticated`. +That word `authenticated` is produced by `service_summaries` (`gitea_mcp_server.py:19713`, defined at `gitea_config.py:837`), whose default check calls `_keychain_token` on the service's own keychain reference (`gitea_config.py:851`). Producing that one line requires the Gitea MCP server to read the Jenkins secret and the GlitchTip secret out of the @@ -195,8 +197,8 @@ keychain. B7 does not exist. **Finding 5 — Jenkins and GlitchTip are already decomposed; the reach is residual.** Their tools live in separately registered servers, marked `external-mcp` -(`gitea_mcp_server.py:17760`, `gitea_mcp_server.py:17766`, `gitea_mcp_server.py:17787`, -`gitea_mcp_server.py:17792`) with their own expected tool sets (`mcp_discoverability.py:9`, +(`gitea_mcp_server.py:17776`, `gitea_mcp_server.py:17782`, `gitea_mcp_server.py:17803`, +`gitea_mcp_server.py:17808`) with their own expected tool sets (`mcp_discoverability.py:9`, `mcp_discoverability.py:17`). The correct decomposition was already chosen. What remains is a leak across it: the credential *references* still live in the Gitea configuration and are still resolved by the Gitea process. #75 bundled these services into one control-plane @@ -207,8 +209,8 @@ GlitchTip, the Sentry bridge runs *inside* the Gitea server, resolving its token process environment (`sentry_incident_bridge.py:190`) and sending it as a bearer header (`sentry_incident_bridge.py:289`). Being an environment variable rather than a keychain item makes it strictly worse: it needs no keychain prompt and is inherited by every subprocess the -server spawns — including the `ps` invocations at `gitea_mcp_server.py:21585` and -`gitea_mcp_server.py:21629`, reached from `gitea_mcp_server.py:21565`. +server spawns — including the `ps` invocations at `gitea_mcp_server.py:21601` and +`gitea_mcp_server.py:21645`, reached from `gitea_mcp_server.py:21581`. **Finding 7 — The highest-value coordination asset has the weakest gate.** A6 is protected by filesystem permissions alone (CR14). Corrupting a lease requires no Gitea credential, @@ -217,8 +219,8 @@ assumes. Every other asset costs an attacker a credential; this one costs nothin local access, which is exactly ADV5's position. **Finding 8 — Provenance authenticates the launch, not the caller.** `server_provenance` is -reported as exactly `client_managed` or `manual_launch` (`gitea_mcp_server.py:19054`), -derived from environment inspection (`gitea_mcp_server.py:15465`) with the recognized-key +reported as exactly `client_managed` or `manual_launch` (`gitea_mcp_server.py:19070`), +derived from environment inspection (`gitea_mcp_server.py:15481`) with the recognized-key allowlist at `gitea_config.py:1172` and the generator that emits the marker at `gitea_config.py:1233`. Every one of those facts is fixed at process start. A client that is trustworthy at launch and compromised a minute later remains `client_managed` for the life @@ -257,7 +259,7 @@ holds the token and calls the API instead of the tool. **D3 — Credential resolution is scoped to the request principal.** A session must resolve its own credential and must have no path to any other principal's. The resolve-every-profile -behavior behind `gitea_mcp_server.py:19362` and `gitea_mcp_server.py:19697` must report +behavior behind `gitea_mcp_server.py:19378` and `gitea_mcp_server.py:19713` must report configured-or-not from configuration alone, without resolving the secret. *Rationale.* Finding 3. An audit surface that proves a credential exists by fetching it is a @@ -334,11 +336,11 @@ The client is attached to the local fleet over stdio. | Boundary | What ADV1 reaches | Stopped by | | -------- | ----------------- | ---------- | -| B1 | Everything the fleet serves. The client *is* the sanctioned launcher: it satisfies the client-managed check (`gitea_mcp_server.py:15465`) by construction, and provenance is never re-verified after launch (Finding 8). | Nothing. The guard authenticates the launch, not the caller. | +| B1 | Everything the fleet serves. The client *is* the sanctioned launcher: it satisfies the client-managed check (`gitea_mcp_server.py:15481`) by construction, and provenance is never re-verified after launch (Finding 8). | Nothing. The guard authenticates the launch, not the caller. | | B2 | All five roles — it is attached to all five namespaces. It can author a PR, approve it from the reviewer namespace, and merge it from the merger namespace. | Only the in-process self-review check, which compares `jcwalker3` (author) against `sysadmin` (reviewer) and **passes**, because Finding 1 made them different accounts while leaving reviewer and merger identical. A9 falls in one sequence of legitimate calls. | | B3 | Every credential in CR1–CR10 via CR13, with no additional prompt — the daemon is already sanctioned, so `assert_keychain_access_allowed` (`mcp_daemon_guard.py:583`) returns immediately. | Nothing. | | B4 | A1 and A2 in full. | Branch protection at the forge, to the extent configured. | -| B5 | The operator's checkout and every worktree, through the author tools (`gitea_mcp_server.py:10940`), plus the shared stderr path at `mcp_server.py:13`. | Nothing; the server's disk is the target disk. | +| B5 | The operator's checkout and every worktree, through the author tools (`gitea_mcp_server.py:10956`), plus the shared stderr path at `mcp_server.py:13`. | Nothing; the server's disk is the target disk. | | B6 | All coordination state — no credential required (CR14). It can forge lease ownership and clear decision locks. | Filesystem permissions, which it already satisfies. | | B7 | Jenkins (A4) and GlitchTip (A5) secrets via Finding 4, and CR11/CR12 from its own environment. | Nothing. | | B8 | Both tenants. | Nothing in-process; only the disabled-context check (`gitea_config.py:704`), which does not apply to enabled contexts. | -- 2.43.7