diff --git a/agent_assembly/client/gateway.py b/agent_assembly/client/gateway.py index 140bd54..51d000b 100644 --- a/agent_assembly/client/gateway.py +++ b/agent_assembly/client/gateway.py @@ -7,6 +7,7 @@ import httpx from agent_assembly.client.dispatch import DispatchToolResult +from agent_assembly.core.transport_security import require_secure_http_url from agent_assembly.exceptions import GatewayError @@ -27,6 +28,7 @@ def __init__( spawned_by_tool: str | None = None, depth: int | None = None, enforcement_mode: str | None = None, + allow_insecure: bool = False, ) -> None: """ Initialize the GatewayClient. @@ -51,6 +53,12 @@ def __init__( ``None`` (the default) lets the gateway apply its server-side default of live enforcement. Stored for reference; registration itself now goes through the native gRPC path (ADR 0004). + allow_insecure: Permit sending the ``Authorization: Bearer`` API key + over plaintext ``http://`` to a **non-loopback** host. Off by + default — a remote ``http://`` base URL with an API key set is + refused with :class:`ValueError`, because the Bearer credential + would travel unencrypted (AAASM-3725). Loopback ``http://`` is + always allowed (local dev gateway). """ self.gateway_url = gateway_url.rstrip("/") self.control_plane_url = control_plane_url.rstrip("/") if control_plane_url else None @@ -63,6 +71,7 @@ def __init__( self.spawned_by_tool = spawned_by_tool self.depth = depth self.enforcement_mode = enforcement_mode + self.allow_insecure = allow_insecure self._client: httpx.Client | None = None @property @@ -73,11 +82,17 @@ def client(self) -> httpx.Client: it falls back to ``gateway_url`` (single-host OSS dev). """ if self._client is None: + base_url = self.control_plane_url or self.gateway_url + require_secure_http_url( + base_url, + has_api_key=bool(self.api_key), + allow_insecure=self.allow_insecure, + ) headers = {} if self.api_key: headers["Authorization"] = f"Bearer {self.api_key}" self._client = httpx.Client( - base_url=self.control_plane_url or self.gateway_url, + base_url=base_url, headers=headers, timeout=self.timeout, ) diff --git a/agent_assembly/core/assembly.py b/agent_assembly/core/assembly.py index a872c8f..e3b679b 100644 --- a/agent_assembly/core/assembly.py +++ b/agent_assembly/core/assembly.py @@ -21,6 +21,7 @@ register_agent, ) from agent_assembly.core.spawn import _SPAWN_CTX +from agent_assembly.core.transport_security import warn_if_insecure_http_url from agent_assembly.exceptions import AssemblyError, ConfigurationError RuntimeMode = Literal["auto", "ebpf", "proxy", "sdk-only"] @@ -173,6 +174,10 @@ def init_assembly( """ gateway_url = resolve_gateway_url(gateway_url) api_key = resolve_api_key(api_key) + # Warn early when the resolved gateway would carry the Bearer API key over + # plaintext http:// to a non-loopback host (AAASM-3725). The control-plane + # URL is the host the credential is actually sent to when set. + warn_if_insecure_http_url(control_plane_url or gateway_url, has_api_key=bool(api_key)) gateway_url, control_plane_url = _validate_inputs( gateway_url=gateway_url, mode=mode, diff --git a/agent_assembly/core/transport_security.py b/agent_assembly/core/transport_security.py new file mode 100644 index 0000000..18808ee --- /dev/null +++ b/agent_assembly/core/transport_security.py @@ -0,0 +1,136 @@ +"""Shared transport-security validation for gateway connections (AAASM-3685/3725). + +The SDK reaches the governance gateway over two transports that both carry +sensitive material: + +* the op-control gRPC stream (``op_control.py``), which carries the agent + identity triple and operator pause / terminate signals; +* the control-plane HTTP API (``client/gateway.py``), which sends the API key + as an ``Authorization: Bearer`` header. + +Neither must travel unencrypted to a **non-loopback** host without an explicit +opt-in. A loopback target is the local dev-mode gateway, where plaintext is the +documented default; anything else is presumed remote and must be encrypted. + +This module centralises the loopback decision and the two enforcement helpers +so the gRPC and HTTP paths agree on what "secure" means (mirrors node-sdk +AAASM-3123). +""" + +from __future__ import annotations + +import warnings +from urllib.parse import urlsplit + +__all__ = [ + "LOOPBACK_HOSTS", + "gateway_host_of", + "is_loopback_target", + "require_secure_grpc_target", + "require_secure_http_url", + "warn_if_insecure_http_url", +] + +# Hosts treated as loopback for the secure-by-default transport decision. +# A loopback gateway is the local dev-mode CP, where plaintext is the documented +# default; anything else is presumed remote and must be encrypted. +LOOPBACK_HOSTS = frozenset({"localhost", "127.0.0.1", "::1", "[::1]"}) + + +def gateway_host_of(gateway_url: str) -> str: + """Extract the bare host from a gateway target. + + Accepts a gRPC ``host:port`` target, a bare host, or a full URL with a + scheme (``http://host:port/path``). IPv6 bracket form (``[::1]:7391``) is + preserved with its brackets so it can be matched against + :data:`LOOPBACK_HOSTS`. The result is lower-cased for case-insensitive + comparison. + """ + target = gateway_url.strip() + + # Full URL with a scheme — let urlsplit pull out the host:port authority. + if "://" in target: + target = urlsplit(target).netloc + + # Strip any userinfo (``user:pass@host``) — only the host matters here. + if "@" in target: + target = target.rsplit("@", 1)[1] + + # Bracketed IPv6 (``[::1]`` or ``[::1]:7391``): keep the brackets, drop the + # trailing ``:port`` that sits *outside* the closing bracket. + if target.startswith("["): + closing = target.find("]") + if closing != -1: + return target[: closing + 1].lower() + return target.lower() + + # Bare IPv6 without brackets (more than one colon) — no port to strip. + if target.count(":") > 1: + return target.lower() + + # host or host:port — drop the port. + return target.rsplit(":", 1)[0].lower() if ":" in target else target.lower() + + +def is_loopback_target(gateway_url: str) -> bool: + """Return True iff ``gateway_url`` points at a loopback host.""" + return gateway_host_of(gateway_url) in LOOPBACK_HOSTS + + +def require_secure_grpc_target(gateway_url: str, *, allow_insecure: bool) -> None: + """Validate that a plaintext gRPC channel to ``gateway_url`` is permitted. + + A loopback target is always allowed (local dev gateway). A non-loopback + target requires the caller to set ``allow_insecure`` — otherwise raise + :class:`ValueError`, because the op-control stream carries the agent + identity and operator control signals. + """ + if is_loopback_target(gateway_url) or allow_insecure: + return + raise ValueError( + f"Refusing to open an insecure (plaintext) gRPC channel to non-loopback " + f"gateway {gateway_url!r}. Use a TLS channel via channel_factory, or pass " + f"allow_insecure=True to explicitly opt in (loopback dev only)." + ) + + +def require_secure_http_url(gateway_url: str, *, has_api_key: bool, allow_insecure: bool) -> None: + """Validate that an ``http://`` control-plane URL is permitted. + + When an API key is set it is sent as a Bearer header, so a plaintext + ``http://`` URL to a non-loopback host would leak the credential. Refuse + unless the target is loopback or ``allow_insecure`` is set. ``https://`` + URLs (and non-http schemes) always pass. + """ + scheme = urlsplit(gateway_url.strip()).scheme.lower() + if scheme != "http": + return + if not has_api_key: + return + if is_loopback_target(gateway_url) or allow_insecure: + return + raise ValueError( + f"Refusing to send an Authorization: Bearer credential over a plaintext " + f"(non-TLS) connection to non-loopback gateway {gateway_url!r}. Use an " + f"https:// endpoint, or pass allow_insecure=True to explicitly opt in " + f"(loopback dev only)." + ) + + +def warn_if_insecure_http_url(gateway_url: str, *, has_api_key: bool) -> None: + """Emit a warning when a non-loopback ``http://`` gateway carries a key. + + Resolution-time advisory counterpart to :func:`require_secure_http_url`: + the resolver knows the URL and whether a key is set before any request is + issued, so it warns early. The hard refusal still happens later in + ``GatewayClient`` (AAASM-3725). ``https://`` and loopback targets are silent. + """ + scheme = urlsplit(gateway_url.strip()).scheme.lower() + if scheme != "http" or not has_api_key or is_loopback_target(gateway_url): + return + warnings.warn( + f"Gateway {gateway_url!r} uses plaintext http:// while an API key is set; " + f"the Authorization: Bearer credential would travel unencrypted. Use " + f"https:// for non-loopback gateways.", + stacklevel=3, + ) diff --git a/agent_assembly/op_control.py b/agent_assembly/op_control.py index e96d892..aaef26f 100644 --- a/agent_assembly/op_control.py +++ b/agent_assembly/op_control.py @@ -39,6 +39,7 @@ import grpc +from agent_assembly.core.transport_security import require_secure_grpc_target from agent_assembly.exceptions import OpTerminatedError from agent_assembly.proto import common_pb2, policy_pb2, policy_pb2_grpc @@ -102,13 +103,24 @@ def connect( team_id: str, agent_id: str, channel_factory: grpc.Channel | None = None, + allow_insecure: bool = False, ) -> OpControlSubscriber: """Open the gRPC channel + subscription stream and start the reader. ``gateway_url`` is the ``host:port`` of the gateway's gRPC endpoint (no scheme; gRPC uses its own). When ``channel_factory`` is supplied - (tests), it's used instead of opening a fresh insecure channel. + (tests, or a TLS channel built by the caller), it's used verbatim and + the loopback / ``allow_insecure`` check below is bypassed — the caller + has taken responsibility for the transport. + + Otherwise a plaintext ``grpc.insecure_channel`` is opened, but only when + the target is loopback (local dev gateway) or ``allow_insecure`` is set. + The op-control stream carries the agent identity triple and operator + pause / terminate signals, so an unencrypted channel to a non-loopback + host is refused with :class:`ValueError` (AAASM-3685). """ + if channel_factory is None: + require_secure_grpc_target(gateway_url, allow_insecure=allow_insecure) channel = channel_factory or grpc.insecure_channel(gateway_url) stub = policy_pb2_grpc.PolicyServiceStub(channel) # type: ignore[no-untyped-call] proto_agent_id = common_pb2.AgentId(org_id=org_id, team_id=team_id, agent_id=agent_id) diff --git a/test/unit/client/test_gateway_endpoints.py b/test/unit/client/test_gateway_endpoints.py index 9bd4ad7..1c86a24 100644 --- a/test/unit/client/test_gateway_endpoints.py +++ b/test/unit/client/test_gateway_endpoints.py @@ -40,7 +40,11 @@ def _patch_post(client: GatewayClient, mock_post: MagicMock) -> Any: def test_http_client_sets_bearer_auth_header_when_api_key_present() -> None: - client = GatewayClient(gateway_url="http://gw.test", agent_id="a", api_key="sekret") + # allow_insecure opts past the AAASM-3725 plaintext-http refusal so this + # test can assert header behavior on a non-loopback http:// base URL. + client = GatewayClient( + gateway_url="http://gw.test", agent_id="a", api_key="sekret", allow_insecure=True + ) try: assert client.client.headers["Authorization"] == "Bearer sekret" finally: @@ -86,3 +90,38 @@ def test_report_edge_raises_gateway_error_on_http_error() -> None: mock_post = MagicMock(return_value=_raising(httpx.ConnectError("down"))) with _patch_post(client, mock_post), pytest.raises(GatewayError, match="Failed to report edge"): client.report_edge("src", "dst", "messages") + + +class TestHttpTransportSecurity: + """AAASM-3725: refuse Bearer API key over plaintext http to a remote host.""" + + def test_bearer_over_http_non_loopback_rejected(self) -> None: + with GatewayClient(gateway_url="http://gw.test", agent_id="a", api_key="k") as client: + with pytest.raises(ValueError, match="Bearer"): + _ = client.client + + def test_bearer_over_http_loopback_allowed(self) -> None: + with GatewayClient( + gateway_url="http://localhost:7391", agent_id="a", api_key="k" + ) as client: + assert client.client.headers["Authorization"] == "Bearer k" + + def test_bearer_over_https_non_loopback_allowed(self) -> None: + with GatewayClient(gateway_url="https://gw.test", agent_id="a", api_key="k") as client: + assert client.client.headers["Authorization"] == "Bearer k" + + def test_http_non_loopback_without_key_allowed(self) -> None: + with GatewayClient(gateway_url="http://gw.test", agent_id="a") as client: + assert "Authorization" not in client.client.headers + + def test_control_plane_url_is_the_validated_target(self) -> None: + # The Bearer header rides the control-plane base URL when set; a remote + # plaintext control-plane URL must be refused even if gateway_url is safe. + with GatewayClient( + gateway_url="https://gw.test", + agent_id="a", + api_key="k", + control_plane_url="http://cp.remote", + ) as client: + with pytest.raises(ValueError, match="Bearer"): + _ = client.client diff --git a/test/unit/core/test_transport_security.py b/test/unit/core/test_transport_security.py new file mode 100644 index 0000000..a63099a --- /dev/null +++ b/test/unit/core/test_transport_security.py @@ -0,0 +1,94 @@ +"""Tests for the shared transport-security validator (AAASM-3685/3725).""" + +from __future__ import annotations + +import warnings + +import pytest + +from agent_assembly.core import transport_security as ts + + +class TestGatewayHostOf: + @pytest.mark.parametrize( + ("target", "expected"), + [ + ("localhost:7391", "localhost"), + ("127.0.0.1:7391", "127.0.0.1"), + ("http://localhost:7391", "localhost"), + ("https://gw.prod.example/path", "gw.prod.example"), + ("gw.prod.example:443", "gw.prod.example"), + ("[::1]:7391", "[::1]"), + ("[::1]", "[::1]"), + ("user:pass@gw.prod.example:443", "gw.prod.example"), + ("LOCALHOST:7391", "localhost"), + ("bare-host", "bare-host"), + ], + ) + def test_extracts_host(self, target: str, expected: str) -> None: + assert ts.gateway_host_of(target) == expected + + +class TestIsLoopbackTarget: + @pytest.mark.parametrize( + "target", + ["localhost:7391", "127.0.0.1:7391", "http://localhost:7391", "[::1]:7391", "::1"], + ) + def test_loopback(self, target: str) -> None: + assert ts.is_loopback_target(target) is True + + @pytest.mark.parametrize( + "target", + ["gw.prod.example:443", "http://gw.test", "10.0.0.5:7391", "example.com"], + ) + def test_non_loopback(self, target: str) -> None: + assert ts.is_loopback_target(target) is False + + +class TestRequireSecureGrpcTarget: + def test_loopback_allowed(self) -> None: + ts.require_secure_grpc_target("localhost:7391", allow_insecure=False) + + def test_non_loopback_rejected_without_opt_in(self) -> None: + with pytest.raises(ValueError, match="insecure"): + ts.require_secure_grpc_target("gw.prod.example:443", allow_insecure=False) + + def test_non_loopback_allowed_with_opt_in(self) -> None: + ts.require_secure_grpc_target("gw.prod.example:443", allow_insecure=True) + + +class TestRequireSecureHttpUrl: + def test_https_always_allowed(self) -> None: + ts.require_secure_http_url("https://gw.test", has_api_key=True, allow_insecure=False) + + def test_http_no_key_allowed(self) -> None: + ts.require_secure_http_url("http://gw.test", has_api_key=False, allow_insecure=False) + + def test_loopback_http_with_key_allowed(self) -> None: + ts.require_secure_http_url("http://localhost:7391", has_api_key=True, allow_insecure=False) + + def test_non_loopback_http_with_key_rejected(self) -> None: + with pytest.raises(ValueError, match="Bearer"): + ts.require_secure_http_url("http://gw.test", has_api_key=True, allow_insecure=False) + + def test_non_loopback_http_with_key_allowed_when_opted_in(self) -> None: + ts.require_secure_http_url("http://gw.test", has_api_key=True, allow_insecure=True) + + +class TestWarnIfInsecureHttpUrl: + def test_warns_on_non_loopback_http_with_key(self) -> None: + with pytest.warns(UserWarning, match="unencrypted"): + ts.warn_if_insecure_http_url("http://gw.test", has_api_key=True) + + @pytest.mark.parametrize( + ("url", "has_key"), + [ + ("https://gw.test", True), + ("http://gw.test", False), + ("http://localhost:7391", True), + ], + ) + def test_silent_otherwise(self, url: str, has_key: bool) -> None: + with warnings.catch_warnings(): + warnings.simplefilter("error") + ts.warn_if_insecure_http_url(url, has_api_key=has_key) diff --git a/test/unit/test_assembly.py b/test/unit/test_assembly.py index 5067110..b087f5d 100644 --- a/test/unit/test_assembly.py +++ b/test/unit/test_assembly.py @@ -655,3 +655,46 @@ def test_init_assembly_enforcement_mode_defaults_to_none_to_preserve_wire_shape( assert context.client.enforcement_mode is None finally: context.shutdown() + + +def test_init_assembly_warns_on_plaintext_http_with_api_key( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """AAASM-3725: a resolved non-loopback http:// gateway + API key warns.""" + monkeypatch.setattr(core_assembly, "_register_adapters", lambda **kwargs: []) + monkeypatch.setattr( + core_assembly, + "_start_network_layer", + lambda **kwargs: ("sdk-only", core_assembly._noop_shutdown), + ) + + with pytest.warns(UserWarning, match="unencrypted"): + context = init_assembly( + gateway_url="http://gw.remote:9999", + api_key="explicit-key", + agent_id="agent-warn", + ) + context.shutdown() + + +def test_init_assembly_no_warning_for_loopback_http_with_api_key( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """AAASM-3725: loopback http:// + API key must not warn (local dev).""" + import warnings + + monkeypatch.setattr(core_assembly, "_register_adapters", lambda **kwargs: []) + monkeypatch.setattr( + core_assembly, + "_start_network_layer", + lambda **kwargs: ("sdk-only", core_assembly._noop_shutdown), + ) + + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + context = init_assembly( + gateway_url="http://localhost:7391", + api_key="explicit-key", + agent_id="agent-loopback", + ) + context.shutdown() diff --git a/test/unit/test_op_control.py b/test/unit/test_op_control.py index d5a819a..60cacd0 100644 --- a/test/unit/test_op_control.py +++ b/test/unit/test_op_control.py @@ -12,6 +12,7 @@ import time from collections.abc import Iterator from queue import Queue +from unittest.mock import patch import pytest @@ -227,3 +228,63 @@ def waiter() -> None: assert done.wait(timeout=2.0) t.join(timeout=1.0) assert not sub.stream_alive() + + +class TestConnectTransportSecurity: + """AAASM-3685: ``connect`` refuses plaintext gRPC to a non-loopback host.""" + + def test_non_loopback_insecure_channel_rejected(self) -> None: + # No channel_factory → connect would open grpc.insecure_channel; the + # transport guard must refuse before that for a remote target. + with ( + patch("agent_assembly.op_control.grpc.insecure_channel") as mock_chan, + pytest.raises(ValueError, match="insecure"), + ): + OpControlSubscriber.connect( + "gateway.prod.example:443", + org_id="o", + team_id="t", + agent_id="a", + ) + mock_chan.assert_not_called() + + def test_non_loopback_allowed_with_opt_in(self) -> None: + stream = _QueueStream() + stub = _FakeStub(stream) + with ( + patch("agent_assembly.op_control.grpc.insecure_channel") as mock_chan, + patch( + "agent_assembly.op_control.policy_pb2_grpc.PolicyServiceStub", + lambda _ch: stub, + ), + ): + sub = OpControlSubscriber.connect( + "gateway.prod.example:443", + org_id="o", + team_id="t", + agent_id="a", + allow_insecure=True, + ) + mock_chan.assert_called_once_with("gateway.prod.example:443") + stream.end() + sub.close() + + def test_loopback_allowed_without_opt_in(self) -> None: + stream = _QueueStream() + stub = _FakeStub(stream) + with ( + patch("agent_assembly.op_control.grpc.insecure_channel") as mock_chan, + patch( + "agent_assembly.op_control.policy_pb2_grpc.PolicyServiceStub", + lambda _ch: stub, + ), + ): + sub = OpControlSubscriber.connect( + "localhost:7391", + org_id="o", + team_id="t", + agent_id="a", + ) + mock_chan.assert_called_once_with("localhost:7391") + stream.end() + sub.close()