Skip to content

Commit 9caa34c

Browse files
committed
fix(codex): reject non-HTTPS remote URLs to prevent cleartext header leakage
1 parent 9d257cc commit 9caa34c

2 files changed

Lines changed: 65 additions & 1 deletion

File tree

src/apm_cli/adapters/client/codex.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import logging
44
import os
55
from pathlib import Path
6+
from urllib.parse import urlparse
67

78
import toml
89

@@ -231,8 +232,19 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
231232
)
232233
return None
233234

235+
remote_url = (remote.get("url") or "").strip()
236+
237+
scheme = urlparse(remote_url).scheme.lower()
238+
if scheme != "https":
239+
_rich_warning(
240+
f"Skipping MCP server '{server_name}' for Codex CLI: remote URL "
241+
f"must use https:// (got {scheme or 'no scheme'}).",
242+
symbol="warning",
243+
)
244+
return None
245+
234246
remote_config = {
235-
"url": (remote.get("url") or "").strip(),
247+
"url": remote_url,
236248
"id": server_info.get("id", ""),
237249
}
238250
http_headers: dict[str, str] = {}

tests/unit/test_mcp_client_factory.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,58 @@ def test_format_server_config_streamable_http_self_defined(self):
326326
self.assertEqual(config["url"], "https://example.com/mcp")
327327
self.assertEqual(config["http_headers"], {"Authorization": "Bearer xyz"})
328328

329+
@patch("apm_cli.adapters.client.codex._rich_warning")
330+
def test_format_server_config_streamable_http_rejects_non_https(self, mock_warn):
331+
"""Non-HTTPS remote URLs are rejected to prevent cleartext header leakage."""
332+
server_info = {
333+
"name": "evil-remote",
334+
"id": "evil-id",
335+
"remotes": [
336+
{
337+
"url": "http://mcp.example.com/mcp",
338+
"transport_type": "streamable-http",
339+
"headers": [{"name": "Authorization", "value": "Bearer secret"}],
340+
}
341+
],
342+
}
343+
344+
result = self.adapter._format_server_config(server_info)
345+
346+
self.assertIsNone(result)
347+
mock_warn.assert_called_once()
348+
warn_message = mock_warn.call_args[0][0]
349+
self.assertIn("evil-remote", warn_message)
350+
self.assertIn("https://", warn_message)
351+
352+
@patch("apm_cli.adapters.client.codex._rich_warning")
353+
@patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference")
354+
def test_configure_mcp_server_http_remote_rejected(self, mock_find_server, mock_warn):
355+
"""End-to-end: an http:// remote URL never lands in the Codex config."""
356+
mock_find_server.return_value = {
357+
"name": "evil-remote",
358+
"id": "evil-id",
359+
"remotes": [
360+
{
361+
"url": "http://mcp.example.com/mcp",
362+
"transport_type": "streamable-http",
363+
"headers": [{"name": "Authorization", "value": "Bearer secret"}],
364+
}
365+
],
366+
"packages": [],
367+
}
368+
369+
result = self.adapter.configure_mcp_server("evil-remote")
370+
371+
self.assertFalse(result)
372+
mock_warn.assert_called_once()
373+
warn_message = mock_warn.call_args[0][0]
374+
self.assertIn("evil-remote", warn_message)
375+
self.assertIn("https://", warn_message)
376+
377+
# Verify no config was written
378+
config = self.adapter.get_current_config()
379+
self.assertNotIn("mcp_servers", config)
380+
329381
@patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference")
330382
def test_configure_mcp_server_streamable_http_writes_toml_entry(self, mock_find_server):
331383
"""End-to-end install of a streamable-HTTP server writes a parseable TOML entry."""

0 commit comments

Comments
 (0)