Skip to content

Commit cb3643d

Browse files
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
1 parent 21b7e8e commit cb3643d

3 files changed

Lines changed: 35 additions & 3 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pip install -e .
5858

5959
## Use With An MCP Client
6060

61-
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.
61+
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).
6262

6363
With `uvx` (recommended — no install needed):
6464

‎src/commit_check_mcp/server.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,9 +619,12 @@ def main(argv: list[str] | None = None) -> None:
619619
this server it is simply true.
620620
"""
621621
import argparse
622-
import os
623622

624623
parser = argparse.ArgumentParser(prog="commit-check-mcp")
624+
try:
625+
default_port = int(os.environ.get("MCP_PORT", "8000"))
626+
except ValueError:
627+
parser.error(f"MCP_PORT must be an integer, got {os.environ['MCP_PORT']!r}")
625628
parser.add_argument(
626629
"--transport",
627630
choices=["stdio", "http"],
@@ -636,11 +639,20 @@ def main(argv: list[str] | None = None) -> None:
636639
parser.add_argument(
637640
"--port",
638641
type=int,
639-
default=int(os.environ.get("MCP_PORT", "8000")),
642+
default=default_port,
640643
help="port for --transport http (default 8000)",
641644
)
642645
args = parser.parse_args(argv)
643646

647+
# argparse does not check `choices` against env-supplied defaults, and a
648+
# typo in MCP_TRANSPORT must not silently fall back to stdio inside a
649+
# container that expects an HTTP listener.
650+
if args.transport not in ("stdio", "http"):
651+
parser.error(
652+
f"argument --transport: invalid choice: {args.transport!r}"
653+
" (choose from 'stdio', 'http')"
654+
)
655+
644656
if args.transport == "http":
645657
mcp.run(
646658
transport="streamable-http",

‎tests/test_server.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,3 +924,23 @@ def test_transport_from_environment(self, monkeypatch: pytest.MonkeyPatch) -> No
924924
def test_rejects_unknown_transport(self) -> None:
925925
with pytest.raises(SystemExit):
926926
server.main(["--transport", "sse"])
927+
928+
def test_rejects_unknown_transport_from_environment(
929+
self, monkeypatch: pytest.MonkeyPatch
930+
) -> None:
931+
"""argparse skips `choices` for env-supplied defaults; a typo in
932+
MCP_TRANSPORT must fail loudly, not silently serve stdio in a
933+
container that expects an HTTP listener."""
934+
monkeypatch.setenv("MCP_TRANSPORT", "htpp")
935+
monkeypatch.setattr(
936+
server.mcp, "run", lambda **kw: pytest.fail("server must not start")
937+
)
938+
with pytest.raises(SystemExit):
939+
server.main([])
940+
941+
def test_rejects_non_integer_port_from_environment(
942+
self, monkeypatch: pytest.MonkeyPatch
943+
) -> None:
944+
monkeypatch.setenv("MCP_PORT", "eight thousand")
945+
with pytest.raises(SystemExit):
946+
server.main([])

0 commit comments

Comments
 (0)