Skip to content

Commit 832d4e7

Browse files
authored
chore(core): Introduce warning when headers are sent over insecure HTTP connections (#544)
* feat: Introduce warning when headers are sent over insecure HTTP connections * chore: delint * refactor: Relocate HTTP security warning logic from transport and tool initialization to `ToolboxTool` invocation. * chore: delint
1 parent 851c7b5 commit 832d4e7

7 files changed

Lines changed: 114 additions & 53 deletions

File tree

packages/toolbox-core/src/toolbox_core/client.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
from .protocol import Protocol, ToolSchema
3131
from .tool import ToolboxTool
3232
from .toolbox_transport import ToolboxTransport
33-
from .utils import identify_auth_requirements, resolve_value
33+
from .utils import identify_auth_requirements, resolve_value, warn_if_http_and_headers
3434

3535

3636
class ToolboxClient:
@@ -101,6 +101,7 @@ def __init__(
101101
raise ValueError(f"Unsupported MCP protocol version: {protocol}")
102102

103103
self.__client_headers = client_headers if client_headers is not None else {}
104+
warn_if_http_and_headers(url, self.__client_headers)
104105

105106
def __parse_tool(
106107
self,
@@ -224,6 +225,8 @@ async def load_tool(
224225
for name, val in self.__client_headers.items()
225226
}
226227

228+
warn_if_http_and_headers(self.__transport.base_url, auth_token_getters)
229+
227230
manifest = await self.__transport.tool_get(name, resolved_headers)
228231

229232
# parse the provided definition to a tool
@@ -299,6 +302,8 @@ async def load_toolset(
299302
for header_name in original_headers
300303
}
301304

305+
warn_if_http_and_headers(self.__transport.base_url, auth_token_getters)
306+
302307
manifest = await self.__transport.tools_list(name, resolved_headers)
303308

304309
tools: list[ToolboxTool] = []

packages/toolbox-core/src/toolbox_core/tool.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
from inspect import Signature
1919
from types import MappingProxyType
2020
from typing import Any, Awaitable, Callable, Mapping, Optional, Sequence, Union
21-
from warnings import warn
2221

2322
from .itransport import ITransport
2423
from .protocol import ParameterSchema
@@ -27,6 +26,7 @@
2726
identify_auth_requirements,
2827
params_to_pydantic_model,
2928
resolve_value,
29+
warn_if_http_and_headers,
3030
)
3131

3232

@@ -272,6 +272,8 @@ async def __call__(self, *args: Any, **kwargs: Any) -> str:
272272
token_getter
273273
)
274274

275+
warn_if_http_and_headers(self.__transport.base_url, headers)
276+
275277
return await self.__transport.tool_invoke(
276278
self.__name__,
277279
payload,

packages/toolbox-core/src/toolbox_core/toolbox_transport.py

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@
1313
# limitations under the License.
1414

1515
from typing import Mapping, Optional
16-
from warnings import warn
1716

1817
from aiohttp import ClientSession
1918

2019
from .itransport import ITransport
2120
from .protocol import ManifestSchema
21+
from .utils import warn_if_http_and_headers
2222

2323

2424
class ToolboxTransport(ITransport):
@@ -70,14 +70,6 @@ async def tools_list(
7070
async def tool_invoke(
7171
self, tool_name: str, arguments: dict, headers: Mapping[str, str]
7272
) -> str:
73-
# ID tokens contain sensitive user information (claims). Transmitting
74-
# these over HTTP exposes the data to interception and unauthorized
75-
# access. Always use HTTPS to ensure secure communication and protect
76-
# user privacy.
77-
if self.base_url.startswith("http://") and headers:
78-
warn(
79-
"Sending data token over HTTP. User data may be exposed. Use HTTPS for secure communication."
80-
)
8173
url = f"{self.__base_url}/api/tool/{tool_name}/invoke"
8274
async with self.__session.post(
8375
url,

packages/toolbox-core/src/toolbox_core/utils.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515

1616
import asyncio
17+
import warnings
1718
from typing import (
1819
Any,
1920
Awaitable,
@@ -43,6 +44,14 @@ def create_func_docstring(description: str, params: Sequence[ParameterSchema]) -
4344
return docstring
4445

4546

47+
def warn_if_http_and_headers(url: str, headers: Mapping[str, Any] | None) -> None:
48+
"""Logs a warning if the url uses HTTP and sensitive headers are present."""
49+
if url.lower().startswith("http://") and headers:
50+
warnings.warn(
51+
"This connection is using HTTP. To prevent credential exposure, please ensure all communication is sent over HTTPS."
52+
)
53+
54+
4655
def identify_auth_requirements(
4756
req_authn_params: Mapping[str, list[str]],
4857
req_authz_tokens: Sequence[str],

packages/toolbox-core/tests/test_tool.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,3 +685,68 @@ async def test_bind_param_chaining(
685685
json={"count": 42, "message": "chained-call"},
686686
headers={},
687687
)
688+
689+
690+
@pytest.mark.asyncio
691+
@pytest.mark.parametrize(
692+
"base_url, headers, should_warn",
693+
[
694+
(
695+
"http://fake-toolbox-server.com",
696+
{"Authorization": "Bearer token"},
697+
True,
698+
),
699+
(
700+
"https://fake-toolbox-server.com",
701+
{"Authorization": "Bearer token"},
702+
False,
703+
),
704+
("http://fake-toolbox-server.com", {}, False),
705+
("http://fake-toolbox-server.com", None, False),
706+
],
707+
)
708+
async def test_tool_call_http_warning(
709+
http_session: ClientSession,
710+
base_url: str,
711+
headers: Mapping[str, str] | None,
712+
should_warn: bool,
713+
):
714+
"""Tests the HTTP security warning logic during tool invocation via __call__."""
715+
url = f"{base_url}/api/tool/{TEST_TOOL_NAME}/invoke"
716+
args = {"param1": "value1"}
717+
response_payload = {"result": "success"}
718+
transport = ToolboxTransport(base_url, http_session)
719+
720+
tool = ToolboxTool(
721+
transport=transport,
722+
name=TEST_TOOL_NAME,
723+
description="A tool",
724+
params=[
725+
ParameterSchema(name="param1", type="string", description="param1 desc")
726+
],
727+
required_authn_params={},
728+
required_authz_tokens=[],
729+
auth_service_token_getters={},
730+
bound_params={},
731+
client_headers=headers if headers is not None else {},
732+
)
733+
734+
with aioresponses() as m:
735+
m.post(url, status=200, payload=response_payload)
736+
737+
if should_warn:
738+
with pytest.warns(
739+
UserWarning,
740+
match="This connection is using HTTP. To prevent credential exposure, please ensure all communication is sent over HTTPS.",
741+
):
742+
await tool(param1="value1")
743+
else:
744+
# Check no warnings fired
745+
with catch_warnings(record=True) as record:
746+
simplefilter("always")
747+
await tool(param1="value1")
748+
749+
warning_messages = [str(w.message) for w in record]
750+
assert not any(
751+
"This connection is using HTTP" in msg for msg in warning_messages
752+
)

packages/toolbox-core/tests/test_toolbox_transport.py

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -154,48 +154,6 @@ async def test_tool_invoke_failure(http_session: ClientSession):
154154
assert str(exc_info.value) == "Invalid arguments"
155155

156156

157-
@pytest.mark.asyncio
158-
@pytest.mark.parametrize(
159-
"base_url, headers, should_warn",
160-
[
161-
(
162-
"http://fake-toolbox-server.com",
163-
{"Authorization": "Bearer token"},
164-
True,
165-
),
166-
(
167-
"https://fake-toolbox-server.com",
168-
{"Authorization": "Bearer token"},
169-
False,
170-
),
171-
("http://fake-toolbox-server.com", {}, False),
172-
("http://fake-toolbox-server.com", None, False),
173-
],
174-
)
175-
async def test_tool_invoke_http_warning(
176-
http_session: ClientSession,
177-
base_url: str,
178-
headers: Optional[Mapping[str, str]],
179-
should_warn: bool,
180-
):
181-
"""Tests the HTTP security warning logic in tool_invoke."""
182-
url = f"{base_url}/api/tool/{TEST_TOOL_NAME}/invoke"
183-
args = {"param1": "value1"}
184-
response_payload = {"result": "success"}
185-
transport = ToolboxTransport(base_url, http_session)
186-
187-
with aioresponses() as m:
188-
m.post(url, status=200, payload=response_payload)
189-
190-
if should_warn:
191-
with pytest.warns(UserWarning, match="Sending data token over HTTP"):
192-
await transport.tool_invoke(TEST_TOOL_NAME, args, headers)
193-
else:
194-
# By not using pytest.warns, we assert that no warnings are raised.
195-
# The test will fail if an unexpected UserWarning occurs.
196-
await transport.tool_invoke(TEST_TOOL_NAME, args, headers)
197-
198-
199157
@pytest.mark.asyncio
200158
async def test_close_does_not_close_unmanaged_session():
201159
"""

packages/toolbox-core/tests/test_utils.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515

1616
import asyncio
17+
import warnings
1718
from typing import Type
1819
from unittest.mock import Mock
1920

@@ -26,6 +27,7 @@
2627
identify_auth_requirements,
2728
params_to_pydantic_model,
2829
resolve_value,
30+
warn_if_http_and_headers,
2931
)
3032

3133

@@ -458,3 +460,31 @@ async def another_async_func():
458460
return {"key": "value"}
459461

460462
assert await resolve_value(another_async_func) == {"key": "value"}
463+
464+
465+
def test_warn_if_http_and_headers_triggers():
466+
"""Test that a warning is emitted for HTTP URLs with headers."""
467+
url = "http://example.com"
468+
headers = {"Authorization": "Bearer token"}
469+
with pytest.warns(UserWarning, match="This connection is using HTTP"):
470+
warn_if_http_and_headers(url, headers)
471+
472+
473+
def test_warn_if_http_and_headers_no_headers():
474+
"""Test that no warning is emitted for HTTP URLs without headers."""
475+
url = "http://example.com"
476+
headers = {}
477+
with warnings.catch_warnings(record=True) as w:
478+
warnings.simplefilter("always")
479+
warn_if_http_and_headers(url, headers)
480+
assert len(w) == 0
481+
482+
483+
def test_warn_if_http_and_headers_https():
484+
"""Test that no warning is emitted for HTTPS URLs."""
485+
url = "https://example.com"
486+
headers = {"Authorization": "Bearer token"}
487+
with warnings.catch_warnings(record=True) as w:
488+
warnings.simplefilter("always")
489+
warn_if_http_and_headers(url, headers)
490+
assert len(w) == 0

0 commit comments

Comments
 (0)