diff --git a/README.md b/README.md index c07991b..dbb31c5 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ pip install -e . ## Use With An MCP Client -This server runs over stdio, so it is meant to be launched by an MCP client rather than used as a long-running HTTP service. +By default this server runs over stdio and is launched by an MCP client. It can also run as a stateless Streamable HTTP service — see [Run as a Stateless HTTP Server](#run-as-a-stateless-http-server). With `uvx` (recommended — no install needed): @@ -230,7 +230,77 @@ commit-check-mcp uvx commit-check-mcp ``` -The server uses stdio transport, which is the recommended MCP default for local tool integrations. +The server uses stdio transport by default, which is the recommended MCP default for local tool integrations. + +## Run as a Stateless HTTP Server + +For remote or containerized deployments, the server can speak Streamable HTTP +in the stateless mode introduced by the 2026-07-28 MCP specification. Every +tool is a pure function of its inputs, so no session handshake is required and +any instance behind a load balancer can answer any request: + +```bash +# Serve on http://127.0.0.1:8000/mcp +commit-check-mcp --transport http + +# Bind all interfaces on a custom port (e.g. inside a container) +commit-check-mcp --transport http --host 0.0.0.0 --port 8080 +``` + +The same settings are available as environment variables for container +images: `COMMIT_CHECK_MCP_TRANSPORT=http`, `COMMIT_CHECK_MCP_HOST`, and +`COMMIT_CHECK_MCP_PORT`. They are deliberately prefixed — an unprefixed +`MCP_TRANSPORT` belongs to no particular server, and a stray value would +turn a stdio launch into an HTTP listener that never answers its client. + +> [!IMPORTANT] +> The server has no built-in authentication or TLS. The `0.0.0.0` bind is +> meant for containers on a private network behind a TLS-terminating +> reverse proxy — do not expose the port to the internet as-is. When the +> server is reachable through a public hostname, restrict the `Host` and +> `Origin` headers it accepts so DNS-rebinding protection stays strict: +> +> ```bash +> commit-check-mcp --transport http --host 0.0.0.0 --port 8080 \ +> --allowed-hosts mcp.example.com,mcp.example.com:443 \ +> --allowed-origins https://app.example.com +> ``` +> +> (also available as `COMMIT_CHECK_MCP_ALLOWED_HOSTS` / +> `COMMIT_CHECK_MCP_ALLOWED_ORIGINS`). `--allowed-origins` requires +> `--allowed-hosts`: an empty host allowlist rejects every request with +> 421, so the server refuses to start on that combination. + +Each request is self-contained — clients can `POST` a `tools/call` directly +to `/mcp` without an `initialize` handshake or `Mcp-Session-Id` header: + +```bash +curl -s -X POST http://127.0.0.1:8000/mcp \ + -H "Content-Type: application/json" \ + -H "Accept: application/json" \ + -H "MCP-Protocol-Version: 2026-07-28" \ + -H "Mcp-Method: tools/call" \ + -H "Mcp-Name: validate_commit_message" \ + -d '{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "validate_commit_message", + "arguments": {"message": "feat: add stateless http transport"}, + "_meta": { + "io.modelcontextprotocol/protocolVersion": "2026-07-28", + "io.modelcontextprotocol/clientCapabilities": {} + } + } + }' +``` + +The `Mcp-Method` and `Mcp-Name` headers let load balancers route on the +call without parsing the body (SEP-2243). Declaring `MCP-Protocol-Version` +commits the request to the 2026-07-28 format, which also requires the +`_meta` envelope shown above — omit both to fall back to the SDK's +backward-compatible minimal form. ## Tool Usage diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 22b0022..433df02 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -608,9 +608,114 @@ def describe_validation_rules( } -def main() -> None: - """Run commit-check MCP server via stdio transport.""" - mcp.run(transport="stdio") +def main(argv: list[str] | None = None) -> None: + """Run the commit-check MCP server. + + Defaults to stdio for local MCP clients. ``--transport http`` serves + stateless Streamable HTTP per the 2026-07-28 MCP specification: every + tool here is a pure function of its inputs, so no request ever depends + on an earlier one and any instance behind a plain load balancer can + answer it. Statelessness is therefore not offered as a toggle — for + this server it is simply true. + """ + import argparse + + # Environment variables carry the COMMIT_CHECK_MCP_ prefix on purpose. A + # bare MCP_TRANSPORT belongs to no particular server, so an unrelated + # value left in the environment would turn a stdio launch — how every + # desktop MCP client starts this server — into an HTTP listener that + # never answers the client's handshake. + parser = argparse.ArgumentParser(prog="commit-check-mcp") + parser.add_argument( + "--transport", + choices=["stdio", "http"], + default=os.environ.get("COMMIT_CHECK_MCP_TRANSPORT", "stdio"), + help="stdio for local clients (default); http for a stateless remote server", + ) + parser.add_argument( + "--host", + default=os.environ.get("COMMIT_CHECK_MCP_HOST", "127.0.0.1"), + help="bind address for --transport http (default 127.0.0.1; use 0.0.0.0 in containers)", + ) + def _port(value: str) -> int: + # Out of range, uvicorn dies deep in asyncio with a bare OverflowError; + # port 0 is worse, binding to an arbitrary free port so the server + # comes up "successfully" somewhere nothing can reach it. + port = int(value) + if not 1 <= port <= 65535: + raise argparse.ArgumentTypeError( + f"port must be between 1 and 65535, got {port}" + ) + return port + + # A string default is converted through the type function only when --port + # is absent, so an invalid inherited port still fails loudly on its own but + # cannot veto an explicit, valid --port. + parser.add_argument( + "--port", + type=_port, + default=os.environ.get("COMMIT_CHECK_MCP_PORT", "8000"), + help="port for --transport http (default 8000)", + ) + parser.add_argument( + "--allowed-hosts", + default=os.environ.get("COMMIT_CHECK_MCP_ALLOWED_HOSTS", ""), + help=( + "comma-separated Host header allowlist for --transport http" + " (e.g. mcp.example.com,mcp.example.com:443); enables strict" + " Host/Origin validation against DNS rebinding" + ), + ) + parser.add_argument( + "--allowed-origins", + default=os.environ.get("COMMIT_CHECK_MCP_ALLOWED_ORIGINS", ""), + help=( + "comma-separated Origin header allowlist for --transport http;" + " requires --allowed-hosts" + ), + ) + args = parser.parse_args(argv) + + # argparse does not check `choices` against env-supplied defaults, and a + # typo in COMMIT_CHECK_MCP_TRANSPORT must not silently fall back to stdio + # inside a container that expects an HTTP listener. + if args.transport not in ("stdio", "http"): + parser.error( + f"argument --transport: invalid choice: {args.transport!r}" + " (choose from 'stdio', 'http')" + ) + + if args.transport == "http": + http_kwargs: dict[str, Any] = { + "host": args.host, + "port": args.port, + "stateless_http": True, + "json_response": True, + } + allowed_hosts = [h.strip() for h in args.allowed_hosts.split(",") if h.strip()] + allowed_origins = [ + o.strip() for o in args.allowed_origins.split(",") if o.strip() + ] + # Turning on DNS-rebinding protection with an empty host allowlist + # rejects *every* request with 421, so origins-only is never a usable + # configuration — fail at startup instead of serving a server that + # answers nothing. + if allowed_origins and not allowed_hosts: + parser.error( + "--allowed-origins requires --allowed-hosts: an empty host" + " allowlist rejects every request with 421 Misdirected Request" + ) + if allowed_hosts: + from mcp.server.transport_security import TransportSecuritySettings + + http_kwargs["transport_security"] = TransportSecuritySettings( + enable_dns_rebinding_protection=True, + allowed_hosts=allowed_hosts, + allowed_origins=allowed_origins, + ) + mcp.run(transport="streamable-http", **http_kwargs) + else: + mcp.run(transport="stdio") if __name__ == "__main__": # pragma: no cover diff --git a/tests/test_server.py b/tests/test_server.py index 6f07456..e635ad7 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -883,5 +883,165 @@ def fake_run(*, transport: str) -> None: assert transport == "stdio" monkeypatch.setattr(server.mcp, "run", fake_run) - server.main() + server.main([]) assert called + + def test_http_transport_is_stateless(self, monkeypatch: pytest.MonkeyPatch) -> None: + """--transport http must serve stateless Streamable HTTP. + + stateless_http and json_response are what let any instance behind a + plain load balancer answer any request under the 2026-07-28 spec; + this pins them so a refactor cannot silently reintroduce sessions. + """ + captured: dict[str, object] = {} + + def fake_run(**kwargs: object) -> None: + captured.update(kwargs) + + monkeypatch.setattr(server.mcp, "run", fake_run) + server.main(["--transport", "http", "--host", "0.0.0.0", "--port", "9000"]) + assert captured == { + "transport": "streamable-http", + "host": "0.0.0.0", + "port": 9000, + "stateless_http": True, + "json_response": True, + } + + def test_transport_from_environment(self, monkeypatch: pytest.MonkeyPatch) -> None: + """COMMIT_CHECK_MCP_TRANSPORT/_HOST/_PORT configure containers without argv.""" + captured: dict[str, object] = {} + monkeypatch.setenv("COMMIT_CHECK_MCP_TRANSPORT", "http") + monkeypatch.setenv("COMMIT_CHECK_MCP_HOST", "0.0.0.0") + monkeypatch.setenv("COMMIT_CHECK_MCP_PORT", "8080") + monkeypatch.setattr(server.mcp, "run", lambda **kw: captured.update(kw)) + server.main([]) + assert captured["transport"] == "streamable-http" + assert captured["host"] == "0.0.0.0" + assert captured["port"] == 8080 + assert captured["stateless_http"] is True + + def test_rejects_unknown_transport(self) -> None: + with pytest.raises(SystemExit): + server.main(["--transport", "sse"]) + + def test_rejects_unknown_transport_from_environment( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """argparse skips `choices` for env-supplied defaults; a typo in + COMMIT_CHECK_MCP_TRANSPORT must fail loudly, not silently serve stdio in a + container that expects an HTTP listener.""" + monkeypatch.setenv("COMMIT_CHECK_MCP_TRANSPORT", "htpp") + monkeypatch.setattr( + server.mcp, "run", lambda **kw: pytest.fail("server must not start") + ) + with pytest.raises(SystemExit): + server.main([]) + + def test_rejects_non_integer_port_from_environment( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("COMMIT_CHECK_MCP_PORT", "eight thousand") + with pytest.raises(SystemExit): + server.main([]) + + def test_cli_port_overrides_invalid_environment_port( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """An invalid inherited COMMIT_CHECK_MCP_PORT must not veto an explicit --port.""" + captured: dict[str, object] = {} + monkeypatch.setenv("COMMIT_CHECK_MCP_PORT", "eight thousand") + monkeypatch.setattr(server.mcp, "run", lambda **kw: captured.update(kw)) + server.main(["--transport", "http", "--port", "9000"]) + assert captured["port"] == 9000 + + def test_allowed_hosts_enable_transport_security( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + captured: dict[str, object] = {} + monkeypatch.setattr(server.mcp, "run", lambda **kw: captured.update(kw)) + server.main( + [ + "--transport", + "http", + "--allowed-hosts", + "mcp.example.com, mcp.example.com:443", + "--allowed-origins", + "https://app.example.com", + ] + ) + security = captured["transport_security"] + assert security.enable_dns_rebinding_protection is True # type: ignore[attr-defined] + assert security.allowed_hosts == [ # type: ignore[attr-defined] + "mcp.example.com", + "mcp.example.com:443", + ] + assert security.allowed_origins == ["https://app.example.com"] # type: ignore[attr-defined] + + def test_no_transport_security_by_default( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + captured: dict[str, object] = {} + monkeypatch.setattr(server.mcp, "run", lambda **kw: captured.update(kw)) + server.main(["--transport", "http"]) + assert "transport_security" not in captured + + @pytest.mark.parametrize("port", ["-1", "0", "65536", "99999"]) + def test_rejects_out_of_range_port( + self, port: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Out of range uvicorn dies with a bare OverflowError; port 0 binds to + an arbitrary free port and comes up where nothing can reach it.""" + monkeypatch.setattr( + server.mcp, "run", lambda **kw: pytest.fail("server must not start") + ) + with pytest.raises(SystemExit): + server.main(["--transport", "http", "--port", port]) + + @pytest.mark.parametrize("port", ["-1", "0", "65536"]) + def test_rejects_out_of_range_port_from_environment( + self, port: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("COMMIT_CHECK_MCP_PORT", port) + monkeypatch.setattr( + server.mcp, "run", lambda **kw: pytest.fail("server must not start") + ) + with pytest.raises(SystemExit): + server.main(["--transport", "http"]) + + @pytest.mark.parametrize("port", ["1", "8000", "65535"]) + def test_accepts_boundary_ports( + self, port: str, monkeypatch: pytest.MonkeyPatch + ) -> None: + captured: dict[str, object] = {} + monkeypatch.setattr(server.mcp, "run", lambda **kw: captured.update(kw)) + server.main(["--transport", "http", "--port", port]) + assert captured["port"] == int(port) + + def test_origins_without_hosts_is_rejected( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Origins-only would enable rebinding protection with an empty host + allowlist, which answers every request with 421 — verified against a + live server. Refuse to start rather than serve a dead endpoint.""" + monkeypatch.setattr( + server.mcp, "run", lambda **kw: pytest.fail("server must not start") + ) + with pytest.raises(SystemExit): + server.main( + ["--transport", "http", "--allowed-origins", "https://app.example.com"] + ) + + def test_unprefixed_env_vars_are_ignored( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A bare MCP_TRANSPORT belongs to no particular server. Honouring it + would turn a stdio launch into an HTTP listener that never answers the + client's handshake.""" + captured: dict[str, object] = {} + monkeypatch.setenv("MCP_TRANSPORT", "http") + monkeypatch.setenv("MCP_PORT", "9999") + monkeypatch.delenv("COMMIT_CHECK_MCP_TRANSPORT", raising=False) + monkeypatch.setattr(server.mcp, "run", lambda **kw: captured.update(kw)) + server.main([]) + assert captured == {"transport": "stdio"}