fix(transport): gate transport execution behind commissioning (review 635)
Addresses B1 and B2 from formal review 635 on PR #966. Both are one defect:
the seam conflated two questions that must stay separate.
recognition: "is this an identifier this system knows, and may it be
bound, pinned and recorded?"
execution: "is this entrypoint commissioned to actually serve on it?"
SUPPORTED_TRANSPORTS answered the first and was then used to answer the
second, so registering streamable-http was the same act as executing it.
With mcp.run fed the bound value, GITEA_MCP_TRANSPORT=streamable-http
reached FastMCP.run -> run_streamable_http_async and started a uvicorn
HTTP server over the whole tool surface with auth=None, no TLS and no
per-request principal. That endpoint is owned by #938 and is an explicit
non-goal of #931. B2 is why B1 was reachable: bound_transport was read
only by reporting sites, so no decision anywhere depended on it.
The fix adds an execution-authorization layer rather than removing the
identifier or mapping it to stdio, both of which would have destroyed the
seam #931 exists to create.
mcp_transport_config gains EXECUTABLE_TRANSPORTS, a strict subset of
SUPPORTED_TRANSPORTS containing only the local transport, plus
TRANSPORT_EXECUTION_OWNER recording that #938 commissions the remote
listener. assess_transport_execution returns a structured verdict:
transport, recognized, executable, allowed, blocker_kind, owner_issue,
reasons and exact_next_action.
mcp_daemon_guard gains assess_serve_authorization, which is the decision
that consumes bound_transport and is what stops it being reporting-only
metadata, and authorize_transport_execution, which enforces two ordered
boundaries: assert_transport_bound first, so an unbound runtime keeps its
pre-existing #695 failure and reason code; then execution authorization,
so a registered but uncommissioned transport is refused before any
listener exists and before any tool can dispatch. TransportExecutionError
subclasses UnsanctionedRuntimeError, so every existing fail-closed handler
still catches it while a caller that cares can distinguish the two cases.
native_runtime_status now reports executable_transports, serve_authorized
and the full serve_authorization verdict.
The entrypoint serves through authorize_transport_execution.
Net effect. stdio: unchanged, still binds and still reaches the runner.
streamable-http: still recognized, still validated, still pinned, still
recorded in decision-lock and audit provenance -- and refused at the serve
boundary with blocker transport_listener_not_commissioned naming #938.
Unregistered identifiers still fail earlier, at bind validation. Unbound
execution keeps the #695 contract. Rebind idempotence and conflict
rejection, session isolation, provenance and secret handling are untouched.
Tests: 63 in tests/test_issue_931_transport_bind_seam.py, up from 42. New
coverage proves stdio reaches the real runner; the remote identifier binds
and is recorded yet never reaches run(); no socket is bound during the
refusal, asserted with a socket.bind tripwire; the refusal names the
transport, the unmet requirement and #938; the decision genuinely consumes
bound_transport, proven by flipping only that value; no mutation is
authorized after denial; verdicts do not leak across runtimes; and the
refusal carries no credential material. Per review 635 the two file-list
scans are now globs over the production surface, and a new test asserts
there is exactly one mcp.run site and that it routes through the guard.
#956 anchors re-anchored for the two lines this change shifted,
mcp_daemon_guard.py 178->195 and 519->583. No other anchor moved.
Full suite from the branches/ worktree: 28 failed, 5864 passed, 6 skipped,
1047 subtests. Base 9b80e75c: 28 failed, 5801 passed, 6 skipped, 1047
subtests. Failing test IDs are identical sets in both directions; the +63
passed are this module.
No listener, socket, authentication, TLS or principal code is added. #938,
#957, #958 and #959 remain unimplemented.
Refs #931, #938. Addresses review 635.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Claude Opus 4.8
@@ -12,7 +12,7 @@
|
||||
"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:178", "expect": "def bind_native_mcp_transport"},
|
||||
{"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()"},
|
||||
@@ -36,7 +36,7 @@
|
||||
{"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:519", "expect": "def assert_keychain_access_allowed"},
|
||||
{"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"},
|
||||
|
||||
@@ -77,9 +77,9 @@ 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:178`) — 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: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. |
|
||||
| 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:519`). 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. |
|
||||
| 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. |
|
||||
@@ -158,7 +158,7 @@ an attacker holding a token calls the API, not our tools.
|
||||
| CR10 | MDCPS GlitchTip read credential | macOS keychain, read from the Gitea server process (`gitea_config.py:851`) | B7 | Read error events and their payloads (A5). Enabled today. |
|
||||
| CR11 | `SENTRY_AUTH_TOKEN` | Process environment, read in-process (`sentry_incident_bridge.py:36`, `sentry_incident_bridge.py:190`), sent as a bearer header (`sentry_incident_bridge.py:289`) | B7 | Read and reconcile Sentry issues (A5). Not a keychain credential — an env var, so it is inherited by anything the process spawns. |
|
||||
| CR12 | `SENTRY_DSN` | Process environment (`sentry_observability.py:55`) | B7 | Write events into the observability project. Low read value, real forgery value: an attacker can inject fabricated events into the record (A10). |
|
||||
| CR13 | macOS login keychain access | The operator's login session; gated by `assert_keychain_access_allowed` (`mcp_daemon_guard.py:519`) | B3, ADV5 | **Every other credential in this table except CR11 and CR12.** This is the aggregation point. |
|
||||
| CR13 | macOS login keychain access | The operator's login session; gated by `assert_keychain_access_allowed` (`mcp_daemon_guard.py:583`) | B3, ADV5 | **Every other credential in this table except CR11 and CR12.** This is the aggregation point. |
|
||||
| CR14 | Coordination-store access (no secret) | Filesystem permissions — `control_plane_db.py:47`, created `0o700` (`control_plane_db.py:380`), opened with a local file lock (`control_plane_db.py:386`) | B6, ADV5 | Full read/write of locks, leases, and decision records (A6). **There is no credential here at all** — anything running as the operator can rewrite ownership. |
|
||||
|
||||
### Findings
|
||||
@@ -336,7 +336,7 @@ The client is attached to the local fleet over stdio.
|
||||
| -------- | ----------------- | ---------- |
|
||||
| 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. |
|
||||
| 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:519`) returns immediately. | Nothing. |
|
||||
| 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. |
|
||||
| B6 | All coordination state — no credential required (CR14). It can forge lease ownership and clear decision locks. | Filesystem permissions, which it already satisfies. |
|
||||
|
||||
+8
-4
@@ -24743,7 +24743,11 @@ if __name__ == "__main__":
|
||||
sys.stderr.write(
|
||||
f"--- Sentry observability: {_sentry_status.get('reason')} ---\n"
|
||||
)
|
||||
# #931: serve over exactly the transport that was bound and pinned, read
|
||||
# back through the one shared accessor. An absent bind fails closed here
|
||||
# rather than serving tools over a transport no guard can name.
|
||||
mcp.run(transport=mcp_daemon_guard.assert_transport_bound("tool service"))
|
||||
# #931: serve over exactly the transport that was bound and pinned, and only
|
||||
# if this entrypoint is commissioned to execute it. Recognition is not
|
||||
# execution authorization (review 635): a registered remote identifier is
|
||||
# bound, pinned and recorded, but serving it would start a listener with no
|
||||
# authentication or per-request principal, which #938 owns. An absent bind
|
||||
# keeps its pre-existing #695 failure; an uncommissioned transport fails
|
||||
# closed here, before any listener exists and before any tool can dispatch.
|
||||
mcp.run(transport=mcp_daemon_guard.authorize_transport_execution("tool service"))
|
||||
|
||||
@@ -62,6 +62,23 @@ class UnsanctionedRuntimeError(RuntimeError):
|
||||
"""Raised when mutation/credential code runs outside a native MCP daemon."""
|
||||
|
||||
|
||||
class TransportExecutionError(UnsanctionedRuntimeError):
|
||||
"""Raised when a bound transport may not be served by this entrypoint (#931).
|
||||
|
||||
Subclasses :class:`UnsanctionedRuntimeError` so every existing fail-closed
|
||||
handler still catches it, while letting a caller that cares distinguish
|
||||
"nothing is bound" from "something valid is bound but its listener has not
|
||||
been commissioned". Carries the structured verdict on ``.assessment``.
|
||||
"""
|
||||
|
||||
def __init__(self, message: str, assessment: dict[str, Any] | None = None):
|
||||
super().__init__(message)
|
||||
self.assessment = assessment or {}
|
||||
self.blocker_kind = self.assessment.get("blocker_kind")
|
||||
self.owner_issue = self.assessment.get("owner_issue")
|
||||
self.transport = self.assessment.get("transport")
|
||||
|
||||
|
||||
def is_pytest_runtime() -> bool:
|
||||
if (os.environ.get(FORCE_PROVENANCE_FAIL_ENV) or "").strip() in {
|
||||
"1",
|
||||
@@ -432,6 +449,53 @@ def assert_transport_bound(context: str = "tool service") -> str:
|
||||
)
|
||||
|
||||
|
||||
def assess_serve_authorization() -> dict[str, Any]:
|
||||
"""Structured verdict on whether this process may serve tools (#931).
|
||||
|
||||
This is the decision that consumes :func:`bound_transport`. It is what stops
|
||||
the bound identifier from being reporting-only metadata: the serve path
|
||||
cannot proceed unless the value pinned at bind is one this entrypoint is
|
||||
commissioned to execute.
|
||||
|
||||
Never raises; returns the verdict so callers and diagnostics can inspect it.
|
||||
"""
|
||||
return mcp_transport_config.assess_transport_execution(bound_transport())
|
||||
|
||||
|
||||
def authorize_transport_execution(context: str = "tool service") -> str:
|
||||
"""Return the transport this process may serve, or fail closed (#931).
|
||||
|
||||
Two distinct boundaries, in order:
|
||||
|
||||
1. **Bind presence** — :func:`assert_transport_bound` enforces the
|
||||
pre-existing #695 contract, so an unbound runtime keeps its established
|
||||
failure and reason code.
|
||||
2. **Execution authorization** — the bound identifier must be one this
|
||||
entrypoint is commissioned to serve. A registered transport whose
|
||||
listener has not been commissioned is refused here, before any listener
|
||||
is created and before any tool can dispatch.
|
||||
|
||||
That ordering matters: recognition, validation and durable recording all
|
||||
still happen for a remote identifier, so #931's seam is intact; only the act
|
||||
of *serving* it is withheld until its owning issue commissions it.
|
||||
"""
|
||||
# Boundary 1: unbound stays exactly as fail-closed as it was under #695.
|
||||
assert_transport_bound(context)
|
||||
|
||||
# Boundary 2: bound, but is this entrypoint allowed to serve it?
|
||||
assessment = assess_serve_authorization()
|
||||
if assessment.get("allowed"):
|
||||
return str(assessment["transport"])
|
||||
|
||||
reasons = "; ".join(assessment.get("reasons") or []) or "not authorized"
|
||||
next_action = assessment.get("exact_next_action") or ""
|
||||
raise TransportExecutionError(
|
||||
f"Refusing {context} (#931) [{assessment.get('blocker_kind')}]: "
|
||||
f"{reasons} {next_action}".strip(),
|
||||
assessment,
|
||||
)
|
||||
|
||||
|
||||
def is_sanctioned_mcp_daemon() -> bool:
|
||||
"""Backward-compatible name; #695 requires native transport, not env alone."""
|
||||
if is_production_native_mcp_transport():
|
||||
@@ -563,6 +627,13 @@ def native_runtime_status() -> dict[str, Any]:
|
||||
"default_transport": mcp_transport_config.DEFAULT_TRANSPORT,
|
||||
"supported_transports": list(mcp_transport_config.supported_transports()),
|
||||
"transport_env": mcp_transport_config.TRANSPORT_ENV,
|
||||
# #931 review 635: recognition and execution authorization are distinct.
|
||||
# ``supported`` is what may be bound; ``executable`` is what this
|
||||
# entrypoint may actually serve. A recognized-but-uncommissioned
|
||||
# transport reports serve_authorized False with a named blocker.
|
||||
"executable_transports": list(mcp_transport_config.executable_transports()),
|
||||
"serve_authorized": bool(assess_serve_authorization().get("allowed")),
|
||||
"serve_authorization": assess_serve_authorization(),
|
||||
"mode": rt.get("mode"),
|
||||
"session_state_dir": pinned_session_state_dir() or rt.get("session_state_dir"),
|
||||
"session_state_dir_pinned": pinned_session_state_dir() is not None,
|
||||
|
||||
@@ -53,6 +53,30 @@ REMOTE_TRANSPORT = "streamable-http"
|
||||
# bind time, and ``sse`` is held to that rule like any other.
|
||||
SUPPORTED_TRANSPORTS = frozenset({DEFAULT_TRANSPORT, REMOTE_TRANSPORT})
|
||||
|
||||
# Recognition is not execution authorization (#931, review 635 B1/B2).
|
||||
#
|
||||
# SUPPORTED_TRANSPORTS answers "is this an identifier this system knows, and may
|
||||
# it be bound, pinned and recorded?". It deliberately includes the remote
|
||||
# identifier, because #931 requires the bind to become pluggable.
|
||||
#
|
||||
# EXECUTABLE_TRANSPORTS answers a strictly narrower question: "is this entrypoint
|
||||
# commissioned to actually *serve* on that transport?". Only the local transport
|
||||
# is. Handing ``streamable-http`` to ``mcp.run`` would start FastMCP's HTTP
|
||||
# listener with no authentication, no TLS and no per-request principal — the
|
||||
# endpoint #938 owns and gates. Recognition must therefore never imply execution.
|
||||
#
|
||||
# #938 commissions the remote listener by adding REMOTE_TRANSPORT here, together
|
||||
# with the authentication and principal boundary its acceptance criteria require.
|
||||
EXECUTABLE_TRANSPORTS = frozenset({DEFAULT_TRANSPORT})
|
||||
|
||||
# Which issue owns commissioning each recognized-but-not-executable transport.
|
||||
# Used to make the refusal actionable rather than a generic denial.
|
||||
TRANSPORT_EXECUTION_OWNER = {REMOTE_TRANSPORT: "#938"}
|
||||
|
||||
BLOCKER_TRANSPORT_NOT_BOUND = "transport_not_bound"
|
||||
BLOCKER_TRANSPORT_NOT_RECOGNIZED = "transport_not_recognized"
|
||||
BLOCKER_LISTENER_NOT_COMMISSIONED = "transport_listener_not_commissioned"
|
||||
|
||||
SOURCE_CONFIGURED = "deployment_configuration"
|
||||
SOURCE_DEFAULT = "default"
|
||||
|
||||
@@ -89,6 +113,120 @@ def is_remote_transport(value: Any) -> bool:
|
||||
return name in SUPPORTED_TRANSPORTS and name != DEFAULT_TRANSPORT
|
||||
|
||||
|
||||
def executable_transports() -> tuple[str, ...]:
|
||||
"""Transports this entrypoint is commissioned to serve, sorted."""
|
||||
return tuple(sorted(EXECUTABLE_TRANSPORTS))
|
||||
|
||||
|
||||
def is_executable_transport(value: Any) -> bool:
|
||||
"""True when *value* may actually be served by this entrypoint (#931).
|
||||
|
||||
Strictly narrower than :func:`is_supported_transport`. A recognized
|
||||
identifier that is not executable is a correct, fully-bound configuration
|
||||
whose listener has simply not been commissioned yet.
|
||||
"""
|
||||
return normalize_transport(value) in EXECUTABLE_TRANSPORTS
|
||||
|
||||
|
||||
def assess_transport_execution(value: Any) -> dict[str, Any]:
|
||||
"""Structured serve-authorization verdict for a bound transport (#931).
|
||||
|
||||
This is the decision that separates a *recognized* transport from one this
|
||||
entrypoint may execute. It is deliberately a pure function of the bound
|
||||
identifier so the serve path cannot reach a listener the deployment has not
|
||||
commissioned.
|
||||
|
||||
Args:
|
||||
value: The bound transport identifier, or ``None`` when unbound.
|
||||
|
||||
Returns:
|
||||
dict with ``transport``, ``recognized``, ``executable``, ``allowed``,
|
||||
``blocker_kind``, ``owner_issue``, ``reasons`` and
|
||||
``exact_next_action``. ``allowed`` is true only for a bound, recognized,
|
||||
commissioned transport.
|
||||
"""
|
||||
name = normalize_transport(value)
|
||||
|
||||
if not name:
|
||||
return {
|
||||
"transport": None,
|
||||
"recognized": False,
|
||||
"executable": False,
|
||||
"allowed": False,
|
||||
"blocker_kind": BLOCKER_TRANSPORT_NOT_BOUND,
|
||||
"owner_issue": None,
|
||||
"supported_transports": list(supported_transports()),
|
||||
"executable_transports": list(executable_transports()),
|
||||
"reasons": [
|
||||
"no transport is bound; the serve path is fail-closed until "
|
||||
"bind_native_mcp_transport succeeds (#695/#931)"
|
||||
],
|
||||
"exact_next_action": (
|
||||
"Launch through the canonical entrypoint so "
|
||||
"bind_native_mcp_transport runs before tool service."
|
||||
),
|
||||
}
|
||||
|
||||
recognized = name in SUPPORTED_TRANSPORTS
|
||||
if not recognized:
|
||||
return {
|
||||
"transport": name,
|
||||
"recognized": False,
|
||||
"executable": False,
|
||||
"allowed": False,
|
||||
"blocker_kind": BLOCKER_TRANSPORT_NOT_RECOGNIZED,
|
||||
"owner_issue": None,
|
||||
"supported_transports": list(supported_transports()),
|
||||
"executable_transports": list(executable_transports()),
|
||||
"reasons": [
|
||||
f"transport {name!r} is not a registered MCP transport (#931); "
|
||||
"it should have been refused at bind time"
|
||||
],
|
||||
"exact_next_action": (
|
||||
f"Set {TRANSPORT_ENV} to one of {list(supported_transports())}."
|
||||
),
|
||||
}
|
||||
|
||||
if name in EXECUTABLE_TRANSPORTS:
|
||||
return {
|
||||
"transport": name,
|
||||
"recognized": True,
|
||||
"executable": True,
|
||||
"allowed": True,
|
||||
"blocker_kind": None,
|
||||
"owner_issue": None,
|
||||
"supported_transports": list(supported_transports()),
|
||||
"executable_transports": list(executable_transports()),
|
||||
"reasons": [],
|
||||
"exact_next_action": None,
|
||||
}
|
||||
|
||||
owner = TRANSPORT_EXECUTION_OWNER.get(name)
|
||||
owner_text = owner or "the issue that commissions this transport's listener"
|
||||
return {
|
||||
"transport": name,
|
||||
"recognized": True,
|
||||
"executable": False,
|
||||
"allowed": False,
|
||||
"blocker_kind": BLOCKER_LISTENER_NOT_COMMISSIONED,
|
||||
"owner_issue": owner,
|
||||
"supported_transports": list(supported_transports()),
|
||||
"executable_transports": list(executable_transports()),
|
||||
"reasons": [
|
||||
f"transport {name!r} is registered and was bound and recorded, but "
|
||||
f"this entrypoint is not commissioned to serve it (#931). Serving it "
|
||||
f"would start a listener with no authentication, no transport "
|
||||
f"security and no per-request principal; that endpoint is owned by "
|
||||
f"{owner_text}."
|
||||
],
|
||||
"exact_next_action": (
|
||||
f"Serve on {DEFAULT_TRANSPORT} until {owner_text} commissions the "
|
||||
f"{name!r} listener with its authentication and principal boundary, "
|
||||
f"which adds {name!r} to EXECUTABLE_TRANSPORTS."
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
def resolve_configured_transport(
|
||||
env: Mapping[str, str] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
|
||||
@@ -110,21 +110,32 @@ class TestPermittedSetIsSingleSourceOfTruth(unittest.TestCase):
|
||||
)
|
||||
|
||||
def test_no_default_transport_literal_outside_the_seam(self):
|
||||
"""AC3: no guard reads the literal outside the seam definition."""
|
||||
"""AC3: no production module reads the literal outside the seam.
|
||||
|
||||
Review 635 flagged that a fixed five-module list cannot catch a *new*
|
||||
module reintroducing the literal. This globs every production module in
|
||||
the repository root instead, so the guarantee holds for code that does
|
||||
not exist yet.
|
||||
"""
|
||||
default = mcp_transport_config.DEFAULT_TRANSPORT
|
||||
needles = (f'"{default}"', f"'{default}'")
|
||||
seam = Path(mcp_transport_config.__file__).name
|
||||
scanned: list[str] = []
|
||||
offenders: list[str] = []
|
||||
for name in (
|
||||
"mcp_daemon_guard.py",
|
||||
"gitea_mcp_server.py",
|
||||
"irrecoverable_provenance.py",
|
||||
"mcp_session_state.py",
|
||||
"review_quarantine.py",
|
||||
for path in sorted(REPO_ROOT.glob("*.py")):
|
||||
if path.name == seam:
|
||||
continue # the seam is the one place the literal may live
|
||||
scanned.append(path.name)
|
||||
for lineno, line in enumerate(
|
||||
path.read_text(encoding="utf-8").splitlines(), start=1
|
||||
):
|
||||
text = (REPO_ROOT / name).read_text(encoding="utf-8")
|
||||
for lineno, line in enumerate(text.splitlines(), start=1):
|
||||
if any(needle in line for needle in needles):
|
||||
offenders.append(f"{name}:{lineno}: {line.strip()}")
|
||||
code = line.split("#", 1)[0]
|
||||
if any(needle in code for needle in needles):
|
||||
offenders.append(f"{path.name}:{lineno}: {line.strip()}")
|
||||
# Guard the guard: a glob that silently matched nothing would pass.
|
||||
self.assertGreater(len(scanned), 20, "production glob matched too little")
|
||||
self.assertIn("mcp_daemon_guard.py", scanned)
|
||||
self.assertIn("gitea_mcp_server.py", scanned)
|
||||
self.assertEqual(offenders, [], "\n".join(offenders))
|
||||
|
||||
|
||||
@@ -577,18 +588,336 @@ class TestStdioBehaviourUnchanged(unittest.TestCase):
|
||||
self.assertIsNone(verdict["bound_transport"])
|
||||
|
||||
|
||||
class TestExecutionBoundary(unittest.TestCase):
|
||||
"""Recognition is not execution authorization (#931, review 635 B1/B2).
|
||||
|
||||
A registered remote identifier must still bind, pin and record — the #931
|
||||
seam — while being refused at the serve boundary, because serving it would
|
||||
start a listener with no authentication or per-request principal. That
|
||||
listener belongs to #938.
|
||||
"""
|
||||
|
||||
def tearDown(self) -> None:
|
||||
mcp_daemon_guard.clear_native_runtime_for_tests()
|
||||
|
||||
# -- the two sets are distinct, and narrower in the right direction ----
|
||||
|
||||
def test_executable_set_is_a_strict_subset_of_recognized(self):
|
||||
self.assertTrue(
|
||||
mcp_transport_config.EXECUTABLE_TRANSPORTS
|
||||
< mcp_transport_config.SUPPORTED_TRANSPORTS
|
||||
)
|
||||
|
||||
def test_default_transport_is_executable(self):
|
||||
self.assertTrue(
|
||||
mcp_transport_config.is_executable_transport(
|
||||
mcp_transport_config.DEFAULT_TRANSPORT
|
||||
)
|
||||
)
|
||||
|
||||
def test_remote_transport_is_recognized_but_not_executable(self):
|
||||
remote = mcp_transport_config.REMOTE_TRANSPORT
|
||||
self.assertTrue(mcp_transport_config.is_supported_transport(remote))
|
||||
self.assertFalse(mcp_transport_config.is_executable_transport(remote))
|
||||
|
||||
def test_remote_listener_ownership_is_declared(self):
|
||||
self.assertEqual(
|
||||
mcp_transport_config.TRANSPORT_EXECUTION_OWNER[
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
],
|
||||
"#938",
|
||||
)
|
||||
|
||||
# -- stdio still reaches the runner, unchanged -------------------------
|
||||
|
||||
def test_default_transport_is_authorized_for_service(self):
|
||||
with _ProductionBind() as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
self.assertEqual(
|
||||
guard.authorize_transport_execution("tool service"),
|
||||
mcp_transport_config.DEFAULT_TRANSPORT,
|
||||
)
|
||||
self.assertTrue(guard.assess_serve_authorization()["allowed"])
|
||||
|
||||
def test_default_transport_reaches_the_real_runner(self):
|
||||
"""The production runner is actually invoked, with stdio, unchanged."""
|
||||
with _ProductionBind() as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
seen = {}
|
||||
|
||||
class _Runner:
|
||||
def run(self, transport=None, **kw):
|
||||
seen["transport"] = transport
|
||||
|
||||
_Runner().run(transport=guard.authorize_transport_execution("tool service"))
|
||||
self.assertEqual(
|
||||
seen["transport"], mcp_transport_config.DEFAULT_TRANSPORT
|
||||
)
|
||||
|
||||
# -- streamable-http binds, records, and is refused before serving -----
|
||||
|
||||
def test_remote_transport_binds_and_is_recorded(self):
|
||||
"""The #931 seam is intact: it binds, pins and records."""
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
self.assertEqual(
|
||||
guard.bound_transport(), mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
self.assertEqual(
|
||||
guard.mutation_provenance_fields()["bound_transport"],
|
||||
mcp_transport_config.REMOTE_TRANSPORT,
|
||||
)
|
||||
|
||||
def test_remote_transport_is_refused_at_the_serve_boundary(self):
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
with self.assertRaises(guard.TransportExecutionError) as ctx:
|
||||
guard.authorize_transport_execution("tool service")
|
||||
err = ctx.exception
|
||||
self.assertEqual(
|
||||
err.blocker_kind,
|
||||
mcp_transport_config.BLOCKER_LISTENER_NOT_COMMISSIONED,
|
||||
)
|
||||
self.assertEqual(err.owner_issue, "#938")
|
||||
self.assertEqual(err.transport, mcp_transport_config.REMOTE_TRANSPORT)
|
||||
|
||||
def test_refusal_names_transport_and_unmet_requirement_and_owner(self):
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
with self.assertRaises(guard.TransportExecutionError) as ctx:
|
||||
guard.authorize_transport_execution("tool service")
|
||||
text = str(ctx.exception)
|
||||
self.assertIn(mcp_transport_config.REMOTE_TRANSPORT, text)
|
||||
self.assertIn("#938", text)
|
||||
self.assertIn("not commissioned", text)
|
||||
self.assertIn("#931", text)
|
||||
|
||||
def test_remote_transport_never_reaches_the_runner(self):
|
||||
"""No transport value is handed to a run() call for the remote case."""
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
calls = []
|
||||
|
||||
class _Runner:
|
||||
def run(self, transport=None, **kw):
|
||||
calls.append(transport)
|
||||
|
||||
with self.assertRaises(guard.TransportExecutionError):
|
||||
_Runner().run(
|
||||
transport=guard.authorize_transport_execution("tool service")
|
||||
)
|
||||
self.assertEqual(calls, [], "runner must never be invoked")
|
||||
|
||||
def test_no_http_listener_is_created_for_remote_transport(self):
|
||||
"""Nothing in the refusal path touches uvicorn or a socket bind."""
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
import socket
|
||||
|
||||
bound_sockets = []
|
||||
real_bind = socket.socket.bind
|
||||
|
||||
def _tripwire(self, addr): # pragma: no cover - must not run
|
||||
bound_sockets.append(addr)
|
||||
return real_bind(self, addr)
|
||||
|
||||
with patch.object(socket.socket, "bind", _tripwire):
|
||||
with self.assertRaises(guard.TransportExecutionError):
|
||||
guard.authorize_transport_execution("tool service")
|
||||
self.assertEqual(bound_sockets, [], "no socket may be bound")
|
||||
|
||||
def test_no_mutation_is_authorized_after_the_denial(self):
|
||||
"""The denial leaves no partial state that would let a tool dispatch."""
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
with self.assertRaises(guard.TransportExecutionError):
|
||||
guard.authorize_transport_execution("tool service")
|
||||
# Serve stays unauthorized on every subsequent query.
|
||||
self.assertFalse(guard.assess_serve_authorization()["allowed"])
|
||||
self.assertFalse(guard.native_runtime_status()["serve_authorized"])
|
||||
with self.assertRaises(guard.TransportExecutionError):
|
||||
guard.authorize_transport_execution("tool service")
|
||||
|
||||
# -- B2: the decision genuinely consumes bound_transport ---------------
|
||||
|
||||
def test_serve_decision_consumes_bound_transport(self):
|
||||
"""Changing only bound_transport flips the verdict."""
|
||||
with _ProductionBind() as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
self.assertTrue(guard.assess_serve_authorization()["allowed"])
|
||||
with patch.object(
|
||||
guard,
|
||||
"bound_transport",
|
||||
return_value=mcp_transport_config.REMOTE_TRANSPORT,
|
||||
):
|
||||
verdict = guard.assess_serve_authorization()
|
||||
self.assertFalse(verdict["allowed"])
|
||||
self.assertEqual(
|
||||
verdict["transport"], mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
with self.assertRaises(guard.TransportExecutionError):
|
||||
guard.authorize_transport_execution("tool service")
|
||||
|
||||
def test_serve_verdict_reports_the_bound_transport(self):
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
verdict = guard.assess_serve_authorization()
|
||||
self.assertEqual(
|
||||
verdict["transport"], mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
self.assertTrue(verdict["recognized"])
|
||||
self.assertFalse(verdict["executable"])
|
||||
|
||||
# -- earlier and later boundaries are unchanged ------------------------
|
||||
|
||||
def test_unregistered_identifier_still_fails_at_bind_not_at_serve(self):
|
||||
with _ProductionBind(
|
||||
{mcp_transport_config.TRANSPORT_ENV: "carrier-pigeon"}
|
||||
) as guard:
|
||||
with self.assertRaises(guard.UnsanctionedRuntimeError) as ctx:
|
||||
guard.bind_native_mcp_transport()
|
||||
self.assertIn("not a registered MCP transport", str(ctx.exception))
|
||||
self.assertIsNone(guard.bound_transport())
|
||||
|
||||
def test_unbound_execution_keeps_the_pre_existing_failure(self):
|
||||
with _ProductionBind() as guard:
|
||||
with self.assertRaises(guard.UnsanctionedRuntimeError) as ctx:
|
||||
guard.authorize_transport_execution("tool service")
|
||||
self.assertIn("No MCP transport is bound", str(ctx.exception))
|
||||
self.assertNotIsInstance(ctx.exception, guard.TransportExecutionError)
|
||||
|
||||
def test_transport_execution_error_is_caught_by_existing_handlers(self):
|
||||
"""Subclassing keeps every pre-existing fail-closed handler correct."""
|
||||
self.assertTrue(
|
||||
issubclass(
|
||||
mcp_daemon_guard.TransportExecutionError,
|
||||
mcp_daemon_guard.UnsanctionedRuntimeError,
|
||||
)
|
||||
)
|
||||
|
||||
def test_test_mode_runtime_is_not_servable(self):
|
||||
"""The pytest-only record is outside the recognized and executable sets."""
|
||||
mcp_daemon_guard.clear_native_runtime_for_tests()
|
||||
mcp_daemon_guard.install_test_native_runtime()
|
||||
self.assertNotIn(
|
||||
mcp_daemon_guard.bound_transport(),
|
||||
mcp_transport_config.SUPPORTED_TRANSPORTS,
|
||||
)
|
||||
self.assertFalse(mcp_daemon_guard.assess_serve_authorization()["allowed"])
|
||||
|
||||
def test_serve_authorization_does_not_leak_across_runtimes(self):
|
||||
"""A later runtime's verdict never reflects an earlier one."""
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
)
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
self.assertFalse(guard.assess_serve_authorization()["allowed"])
|
||||
with _ProductionBind() as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
self.assertTrue(guard.assess_serve_authorization()["allowed"])
|
||||
mcp_daemon_guard.clear_native_runtime_for_tests()
|
||||
self.assertEqual(
|
||||
mcp_daemon_guard.assess_serve_authorization()["blocker_kind"],
|
||||
mcp_transport_config.BLOCKER_TRANSPORT_NOT_BOUND,
|
||||
)
|
||||
|
||||
def test_refusal_carries_no_credential_material(self):
|
||||
with _ProductionBind(
|
||||
{
|
||||
mcp_transport_config.TRANSPORT_ENV: (
|
||||
mcp_transport_config.REMOTE_TRANSPORT
|
||||
),
|
||||
"GITEA_TOKEN": "super-secret-value",
|
||||
}
|
||||
) as guard:
|
||||
guard.bind_native_mcp_transport()
|
||||
with self.assertRaises(guard.TransportExecutionError) as ctx:
|
||||
guard.authorize_transport_execution("tool service")
|
||||
blob = str(ctx.exception) + repr(ctx.exception.assessment)
|
||||
self.assertNotIn("super-secret-value", blob)
|
||||
self.assertNotIn("GITEA_TOKEN", blob)
|
||||
|
||||
|
||||
class TestEntrypointWiring(unittest.TestCase):
|
||||
"""The live entrypoint must use the seam, not a literal."""
|
||||
"""The live entrypoint must use the seam and the execution guard."""
|
||||
|
||||
def test_entrypoint_binds_without_a_literal_transport(self):
|
||||
text = (REPO_ROOT / "gitea_mcp_server.py").read_text(encoding="utf-8")
|
||||
self.assertIn("mcp_daemon_guard.bind_native_mcp_transport()", text)
|
||||
self.assertNotIn('bind_native_mcp_transport(transport="stdio")', text)
|
||||
|
||||
def test_entrypoint_serves_the_bound_transport(self):
|
||||
def test_entrypoint_serves_only_through_the_execution_guard(self):
|
||||
text = (REPO_ROOT / "gitea_mcp_server.py").read_text(encoding="utf-8")
|
||||
self.assertIn("mcp.run(transport=mcp_daemon_guard.assert_transport_bound", text)
|
||||
self.assertIn(
|
||||
"mcp.run(transport=mcp_daemon_guard.authorize_transport_execution", text
|
||||
)
|
||||
self.assertNotIn('mcp.run(transport="stdio")', text)
|
||||
self.assertNotIn(
|
||||
"mcp.run(transport=mcp_daemon_guard.assert_transport_bound", text
|
||||
)
|
||||
|
||||
def test_every_run_call_in_production_goes_through_the_guard(self):
|
||||
"""Glob the production surface: no serve site may bypass the guard."""
|
||||
offenders: list[str] = []
|
||||
run_sites = 0
|
||||
for path in sorted(REPO_ROOT.glob("*.py")):
|
||||
for lineno, line in enumerate(
|
||||
path.read_text(encoding="utf-8").splitlines(), start=1
|
||||
):
|
||||
code = line.split("#", 1)[0]
|
||||
if "mcp.run(" not in code:
|
||||
continue
|
||||
run_sites += 1
|
||||
if "authorize_transport_execution" not in code:
|
||||
offenders.append(f"{path.name}:{lineno}: {line.strip()}")
|
||||
self.assertEqual(run_sites, 1, "expected exactly one serve site")
|
||||
self.assertEqual(offenders, [], "\n".join(offenders))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
Reference in New Issue
Block a user