From 21b7e8eec5cb20d7fc5c7fea740f435e02cb3426 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 17:22:41 +0000 Subject: [PATCH 1/5] feat: add stateless streamable http transport Add a --transport flag (with MCP_TRANSPORT/MCP_HOST/MCP_PORT environment fallbacks) so the server can run as a stateless Streamable HTTP service per the 2026-07-28 MCP specification, alongside the stdio default. Every tool here is a pure function of its inputs, so statelessness is not a toggle: HTTP mode always runs with stateless_http=True and json_response=True, letting any instance behind a plain load balancer answer any request without an initialize handshake or session header. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn --- README.md | 38 +++++++++++++++++++++++++++- src/commit_check_mcp/server.py | 46 +++++++++++++++++++++++++++++++--- tests/test_server.py | 41 +++++++++++++++++++++++++++++- 3 files changed, 120 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c07991b..aa132eb 100644 --- a/README.md +++ b/README.md @@ -230,7 +230,43 @@ 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: `MCP_TRANSPORT=http`, `MCP_HOST`, and `MCP_PORT`. + +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" \ + -d '{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "validate_commit_message", + "arguments": {"message": "feat: add stateless http transport"} + } + }' +``` ## Tool Usage diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 22b0022..c09a832 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -608,9 +608,49 @@ 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 + import os + + parser = argparse.ArgumentParser(prog="commit-check-mcp") + parser.add_argument( + "--transport", + choices=["stdio", "http"], + default=os.environ.get("MCP_TRANSPORT", "stdio"), + help="stdio for local clients (default); http for a stateless remote server", + ) + parser.add_argument( + "--host", + default=os.environ.get("MCP_HOST", "127.0.0.1"), + help="bind address for --transport http (default 127.0.0.1; use 0.0.0.0 in containers)", + ) + parser.add_argument( + "--port", + type=int, + default=int(os.environ.get("MCP_PORT", "8000")), + help="port for --transport http (default 8000)", + ) + args = parser.parse_args(argv) + + if args.transport == "http": + mcp.run( + transport="streamable-http", + host=args.host, + port=args.port, + stateless_http=True, + json_response=True, + ) + 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..5e982fd 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -883,5 +883,44 @@ 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: + """MCP_TRANSPORT/MCP_HOST/MCP_PORT configure containers without argv.""" + captured: dict[str, object] = {} + monkeypatch.setenv("MCP_TRANSPORT", "http") + monkeypatch.setenv("MCP_HOST", "0.0.0.0") + monkeypatch.setenv("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"]) From cb3643da458ac1be335571eef0de3540ef95c75d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 18:06:54 +0000 Subject: [PATCH 2/5] fix: reject invalid transport and port from environment Self-review fixes on top of the stateless HTTP transport: - argparse never checks `choices` against an env-supplied default, so a typo in MCP_TRANSPORT would silently fall back to stdio inside a container that expects an HTTP listener; validate it after parsing. - A non-integer MCP_PORT now exits with a clean parser error instead of an unhandled ValueError traceback. - Drop a redundant local `import os` (already imported at module level). - Fix a stale README sentence claiming the server is not meant to run as a long-running HTTP service, which contradicted the new section. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn --- README.md | 2 +- src/commit_check_mcp/server.py | 16 ++++++++++++++-- tests/test_server.py | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index aa132eb..0bcbf07 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): diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index c09a832..6914de1 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -619,9 +619,12 @@ def main(argv: list[str] | None = None) -> None: this server it is simply true. """ import argparse - import os parser = argparse.ArgumentParser(prog="commit-check-mcp") + try: + default_port = int(os.environ.get("MCP_PORT", "8000")) + except ValueError: + parser.error(f"MCP_PORT must be an integer, got {os.environ['MCP_PORT']!r}") parser.add_argument( "--transport", choices=["stdio", "http"], @@ -636,11 +639,20 @@ def main(argv: list[str] | None = None) -> None: parser.add_argument( "--port", type=int, - default=int(os.environ.get("MCP_PORT", "8000")), + default=default_port, help="port for --transport http (default 8000)", ) args = parser.parse_args(argv) + # argparse does not check `choices` against env-supplied defaults, and a + # typo in 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": mcp.run( transport="streamable-http", diff --git a/tests/test_server.py b/tests/test_server.py index 5e982fd..d78eb84 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -924,3 +924,23 @@ def test_transport_from_environment(self, monkeypatch: pytest.MonkeyPatch) -> No 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 + MCP_TRANSPORT must fail loudly, not silently serve stdio in a + container that expects an HTTP listener.""" + monkeypatch.setenv("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("MCP_PORT", "eight thousand") + with pytest.raises(SystemExit): + server.main([]) From 17d69329a3c466c8186366f3e0c4b0ade1f1c787 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 18:17:32 +0000 Subject: [PATCH 3/5] fix: address review on http transport security and port parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit review on the stateless HTTP transport: - Add --allowed-hosts/--allowed-origins (MCP_ALLOWED_HOSTS / MCP_ALLOWED_ORIGINS) wiring TransportSecuritySettings with strict DNS rebinding protection into HTTP mode; verified live that a foreign Host header is rejected with 421 when an allowlist is set. README now states the server has no built-in auth/TLS and that non-loopback binds belong behind a reverse proxy on a private network. - Parse MCP_PORT lazily via the argparse string-default rule so an invalid inherited MCP_PORT still fails loudly on its own but cannot veto an explicit valid --port; regression test added. - Extend the README curl example with the SEP-2243 headers plus the params._meta envelope that MCP-Protocol-Version requires — the header-only form suggested in review is rejected with 400 by the SDK; the documented form was run verbatim against a live server. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn --- README.md | 30 ++++++++++++++++++++- src/commit_check_mcp/server.py | 49 +++++++++++++++++++++++++--------- tests/test_server.py | 41 ++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 0bcbf07..beacec8 100644 --- a/README.md +++ b/README.md @@ -250,6 +250,21 @@ commit-check-mcp --transport http --host 0.0.0.0 --port 8080 The same settings are available as environment variables for container images: `MCP_TRANSPORT=http`, `MCP_HOST`, and `MCP_PORT`. +> [!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 `MCP_ALLOWED_HOSTS` / `MCP_ALLOWED_ORIGINS`) + Each request is self-contained — clients can `POST` a `tools/call` directly to `/mcp` without an `initialize` handshake or `Mcp-Session-Id` header: @@ -257,17 +272,30 @@ to `/mcp` without an `initialize` handshake or `Mcp-Session-Id` header: 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"} + "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 After the client starts the server, it will expose these tools: diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 6914de1..5f09e13 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -621,10 +621,6 @@ def main(argv: list[str] | None = None) -> None: import argparse parser = argparse.ArgumentParser(prog="commit-check-mcp") - try: - default_port = int(os.environ.get("MCP_PORT", "8000")) - except ValueError: - parser.error(f"MCP_PORT must be an integer, got {os.environ['MCP_PORT']!r}") parser.add_argument( "--transport", choices=["stdio", "http"], @@ -636,12 +632,29 @@ def main(argv: list[str] | None = None) -> None: default=os.environ.get("MCP_HOST", "127.0.0.1"), help="bind address for --transport http (default 127.0.0.1; use 0.0.0.0 in containers)", ) + # A string default is converted through type=int only when --port is + # absent, so an invalid inherited MCP_PORT still fails loudly on its own + # but cannot veto an explicit, valid --port. parser.add_argument( "--port", type=int, - default=default_port, + default=os.environ.get("MCP_PORT", "8000"), help="port for --transport http (default 8000)", ) + parser.add_argument( + "--allowed-hosts", + default=os.environ.get("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("MCP_ALLOWED_ORIGINS", ""), + help="comma-separated Origin header allowlist for --transport http", + ) args = parser.parse_args(argv) # argparse does not check `choices` against env-supplied defaults, and a @@ -654,13 +667,25 @@ def main(argv: list[str] | None = None) -> None: ) if args.transport == "http": - mcp.run( - transport="streamable-http", - host=args.host, - port=args.port, - stateless_http=True, - json_response=True, - ) + 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() + ] + if allowed_hosts or allowed_origins: + 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") diff --git a/tests/test_server.py b/tests/test_server.py index d78eb84..ccd9c0e 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -944,3 +944,44 @@ def test_rejects_non_integer_port_from_environment( monkeypatch.setenv("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 MCP_PORT must not veto an explicit --port.""" + captured: dict[str, object] = {} + monkeypatch.setenv("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 From fea8a7e2267f796aa1e165fa8f9ea073c4925ee7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 04:29:41 +0000 Subject: [PATCH 4/5] fix: prefix env vars and reject origins without hosts Two defects found by testing the merge risk of this branch against a live server rather than only through mocks. --allowed-origins without --allowed-hosts enabled DNS-rebinding protection with an empty host allowlist, which rejects *every* request with 421 Misdirected Request. Measured on a running server: no allowlist -> 200, origins-only -> 421, hosts set -> 200. Origins-only is therefore never a usable configuration, and the README presented both flags side by side, so reaching it took only using one of them. The combination now fails at startup instead of serving an endpoint that answers nothing. The environment variables were unprefixed. A bare MCP_TRANSPORT belongs to no particular server, so a value left over from an unrelated one turned a stdio launch -- how every desktop MCP client starts this server -- into an HTTP listener that never answers the client's handshake; confirmed by launching with MCP_TRANSPORT=http and watching uvicorn come up on a stdio invocation. They are now COMMIT_CHECK_MCP_*, and a test pins that the unprefixed names are ignored. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn --- README.md | 10 ++++++-- src/commit_check_mcp/server.py | 39 ++++++++++++++++++++-------- tests/test_server.py | 46 +++++++++++++++++++++++++++------- 3 files changed, 73 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index beacec8..dbb31c5 100644 --- a/README.md +++ b/README.md @@ -248,7 +248,10 @@ commit-check-mcp --transport http --host 0.0.0.0 --port 8080 ``` The same settings are available as environment variables for container -images: `MCP_TRANSPORT=http`, `MCP_HOST`, and `MCP_PORT`. +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 @@ -263,7 +266,10 @@ images: `MCP_TRANSPORT=http`, `MCP_HOST`, and `MCP_PORT`. > --allowed-origins https://app.example.com > ``` > -> (also available as `MCP_ALLOWED_HOSTS` / `MCP_ALLOWED_ORIGINS`) +> (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: diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 5f09e13..b076f49 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -620,30 +620,35 @@ def main(argv: list[str] | None = None) -> None: """ 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("MCP_TRANSPORT", "stdio"), + 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("MCP_HOST", "127.0.0.1"), + 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)", ) # A string default is converted through type=int only when --port is - # absent, so an invalid inherited MCP_PORT still fails loudly on its own - # but cannot veto an explicit, valid --port. + # absent, so an invalid inherited port still fails loudly on its own but + # cannot veto an explicit, valid --port. parser.add_argument( "--port", type=int, - default=os.environ.get("MCP_PORT", "8000"), + 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("MCP_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" @@ -652,14 +657,17 @@ def main(argv: list[str] | None = None) -> None: ) parser.add_argument( "--allowed-origins", - default=os.environ.get("MCP_ALLOWED_ORIGINS", ""), - help="comma-separated Origin header allowlist for --transport http", + 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 MCP_TRANSPORT must not silently fall back to stdio inside a - # container that expects an HTTP listener. + # 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}" @@ -677,7 +685,16 @@ def main(argv: list[str] | None = None) -> None: allowed_origins = [ o.strip() for o in args.allowed_origins.split(",") if o.strip() ] - if allowed_hosts or allowed_origins: + # 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( diff --git a/tests/test_server.py b/tests/test_server.py index ccd9c0e..d4933de 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -909,11 +909,11 @@ def fake_run(**kwargs: object) -> None: } def test_transport_from_environment(self, monkeypatch: pytest.MonkeyPatch) -> None: - """MCP_TRANSPORT/MCP_HOST/MCP_PORT configure containers without argv.""" + """COMMIT_CHECK_MCP_TRANSPORT/_HOST/_PORT configure containers without argv.""" captured: dict[str, object] = {} - monkeypatch.setenv("MCP_TRANSPORT", "http") - monkeypatch.setenv("MCP_HOST", "0.0.0.0") - monkeypatch.setenv("MCP_PORT", "8080") + 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" @@ -929,9 +929,9 @@ def test_rejects_unknown_transport_from_environment( self, monkeypatch: pytest.MonkeyPatch ) -> None: """argparse skips `choices` for env-supplied defaults; a typo in - MCP_TRANSPORT must fail loudly, not silently serve stdio in a + COMMIT_CHECK_MCP_TRANSPORT must fail loudly, not silently serve stdio in a container that expects an HTTP listener.""" - monkeypatch.setenv("MCP_TRANSPORT", "htpp") + monkeypatch.setenv("COMMIT_CHECK_MCP_TRANSPORT", "htpp") monkeypatch.setattr( server.mcp, "run", lambda **kw: pytest.fail("server must not start") ) @@ -941,16 +941,16 @@ def test_rejects_unknown_transport_from_environment( def test_rejects_non_integer_port_from_environment( self, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.setenv("MCP_PORT", "eight thousand") + 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 MCP_PORT must not veto an explicit --port.""" + """An invalid inherited COMMIT_CHECK_MCP_PORT must not veto an explicit --port.""" captured: dict[str, object] = {} - monkeypatch.setenv("MCP_PORT", "eight thousand") + 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 @@ -985,3 +985,31 @@ def test_no_transport_security_by_default( monkeypatch.setattr(server.mcp, "run", lambda **kw: captured.update(kw)) server.main(["--transport", "http"]) assert "transport_security" not in captured + + 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"} From e4dc39aed33e56422cb4fcb3f37b3067cacaeb65 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 04:34:59 +0000 Subject: [PATCH 5/5] fix: reject http ports outside the valid range argparse's type=int accepted any integer and handed it to uvicorn. Measured against a live server: -1, 65536 and 99999 die deep in asyncio with a bare OverflowError traceback, and port 0 is worse -- the server starts on an arbitrary free port, so a container whose COMMIT_CHECK_MCP_PORT expands to 0 comes up "successfully" somewhere nothing can reach it. That is the same silent misconfiguration this branch already closed for the transport name and the origins-only allowlist. Ports are now validated to 1..65535 by the argument type, which keeps the lazy-conversion property: an invalid inherited environment value still fails on its own but cannot veto an explicit, valid --port. Boundary tests cover both the flag and the environment variable. Raised by CodeRabbit review. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn --- src/commit_check_mcp/server.py | 17 ++++++++++++++--- tests/test_server.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index b076f49..433df02 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -637,12 +637,23 @@ def main(argv: list[str] | None = None) -> None: 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)", ) - # A string default is converted through type=int only when --port is - # absent, so an invalid inherited port still fails loudly on its own but + 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=int, + type=_port, default=os.environ.get("COMMIT_CHECK_MCP_PORT", "8000"), help="port for --transport http (default 8000)", ) diff --git a/tests/test_server.py b/tests/test_server.py index d4933de..e635ad7 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -986,6 +986,38 @@ def test_no_transport_security_by_default( 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: