diff --git a/CHANGELOG.md b/CHANGELOG.md index 1630c32d4..27af4cf82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,16 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Pin Noema's HTTP and HTTPS credential egress to the prevalidated numeric DNS + addresses while preserving the original HTTPS hostname for certificate + validation, preventing DNS rebinding between endpoint validation and socket + creation. +- Fail closed before sending Noema's bearer credential to a configured model + endpoint unless a non-loopback target uses HTTPS with stable, globally + routable unicast DNS evidence; preserve only the literal same-job loopback + sidecar exception, keep redirects disabled, and bound response bodies to + 1 MiB before JSON decoding. + - Publish only the sanitized cumulative Strix report tree, avoiding a later copy of relative scanner output that could reintroduce known internal warning text into uploaded security evidence. diff --git a/docs/doctoring/noema-credential-egress-boundary.md b/docs/doctoring/noema-credential-egress-boundary.md new file mode 100644 index 000000000..e794e0221 --- /dev/null +++ b/docs/doctoring/noema-credential-egress-boundary.md @@ -0,0 +1,57 @@ +# Noema credential-egress boundary + +## Customer outcome + +Noema now rejects a misconfigured or attacker-influenced model endpoint before +placing its bearer credential on the request. A public service must use HTTPS, +must resolve entirely to globally routable unicast addresses, and must retain +the same complete DNS address set after the bounded response is received. +Resolver failures, empty answers, malformed addresses, redirects, special-use +addresses, and response bodies larger than 1 MiB fail closed. + +The existing same-job contextual-orchestrator seam remains usable without +importing the orchestrator into this repository: literal `127.0.0.1` and `::1` +endpoints may use HTTP only when every resolver result is loopback. Hostnames +that merely resolve to loopback do not receive this exception. Provider routing, +model selection, and model-parameter translation remain upstream concerns. + +## Decision and trust boundary + +The implementation reuses Python's `urllib.parse`, `socket.getaddrinfo`, and +`ipaddress` rather than adding a URL or address-classification dependency. +Before request construction it: + +1. rejects non-HTTP schemes, URL user information, and missing hostnames; +2. restricts plaintext HTTP to the two literal loopback sidecar addresses; +3. resolves the endpoint for its effective port and rejects failed, empty, or + malformed resolution evidence; and +4. requires every non-loopback address to be globally routable unicast. + +Redirects remain disabled, and the opener disables ambient proxies. Custom HTTP +and HTTPS connections connect only to the validated numeric addresses while +retaining the original hostname for HTTPS certificate validation. The response +reader requests at most one byte beyond the 1 MiB contract, rejects an +over-limit result before decoding JSON, and then re-resolves the same host and +port. A changed address set invalidates the result. Tests cover IPv4 and IPv6 +loopback, dual-stack public endpoints, pinned numeric TCP destinations, TLS SNI, +DNS rebinding evidence, resolver failures, malformed results, URL credentials, +special-use address classes, and the byte limit. + +Trusted DNS and network egress controls remain defense-in-depth for production; +the application boundary now also prevents the request socket from performing +an unvalidated hostname resolution. + +## References + +Cotton, M., Vegoda, L., Bonica, R., & Haberman, B. (2013). *Special-purpose IP +address registries* (RFC 6890). RFC Editor. https://doi.org/10.17487/RFC6890 + +MITRE. (2026a). *CWE-400: Uncontrolled resource consumption*. +https://cwe.mitre.org/data/definitions/400.html + +MITRE. (2026b). *CWE-918: Server-side request forgery (SSRF)*. +https://cwe.mitre.org/data/definitions/918.html + +OWASP Foundation. (n.d.). *Server-side request forgery prevention cheat sheet*. +Retrieved August 24, 2026, from +https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 9317860e4..3fb52626e 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -5,6 +5,7 @@ import argparse import base64 +import http.client import ipaddress import json import os @@ -41,6 +42,11 @@ MAX_FILE_CONTEXT_CHARS = 4000 MAX_REVIEW_CONTEXT_CHARS = 24000 MAX_THREAD_BODY_CHARS = 1200 +MAX_LLM_RESPONSE_BYTES = 1024 * 1024 +IpAddress = ipaddress.IPv4Address | ipaddress.IPv6Address +TRUSTED_LOOPBACK_ADDRESSES = frozenset( + {ipaddress.ip_address("127.0.0.1"), ipaddress.ip_address("::1")} +) # ⚡ Bolt: Pre-compiled regex patterns to avoid recompilation on every scrub_sensitive_data call. # Impact: Improves string processing performance in error reporting. @@ -423,6 +429,172 @@ def redirect_request( raise urllib.error.HTTPError(req.full_url, code, msg, headers, fp) +def _socket_target(address: IpAddress, port: int) -> tuple[str, int]: + """Return a socket destination that contains only a validated IP literal.""" + return (str(address), port) + + +class _PinnedConnectionMixin: + """Connect only to the DNS addresses validated before the request.""" + + def __init__( + self, + *args: Any, + validated_addresses: frozenset[IpAddress], + **kwargs: Any, + ) -> None: + """Store immutable DNS evidence before initializing the HTTP connection.""" + if not validated_addresses: + raise ValueError("Noema endpoint has no validated DNS addresses") + self._validated_addresses = tuple( + sorted(validated_addresses, key=lambda address: (address.version, address.packed)) + ) + super().__init__(*args, **kwargs) + + def _connect_to_validated_address(self) -> None: + """Open a socket using a validated numeric destination, never the hostname.""" + last_error: OSError | None = None + for address in self._validated_addresses: + try: + self.sock = socket.create_connection( + _socket_target(address, self.port), + self.timeout, + self.source_address, + ) + return + except OSError as exc: + last_error = exc + raise OSError("Noema endpoint could not connect to validated DNS addresses") from last_error + + +class PinnedHTTPConnection(_PinnedConnectionMixin, http.client.HTTPConnection): + """HTTP connection that cannot resolve the configured hostname a second time.""" + + def connect(self) -> None: + """Connect to a validated address and preserve proxy tunnel behavior.""" + self._connect_to_validated_address() + if self._tunnel_host: + self._tunnel() + + +class PinnedHTTPSConnection(_PinnedConnectionMixin, http.client.HTTPSConnection): + """HTTPS connection pinned to validated addresses while retaining hostname TLS.""" + + def connect(self) -> None: + """Connect to a validated address, then verify the original hostname in TLS.""" + self._connect_to_validated_address() + if self._tunnel_host: + self._tunnel() + server_hostname = self._tunnel_host or self.host + self.sock = self._context.wrap_socket(self.sock, server_hostname=server_hostname) + + +class PinnedHTTPHandler(urllib.request.HTTPHandler): + """urllib handler that uses validated numeric destinations for HTTP requests.""" + + def __init__(self, addresses: frozenset[IpAddress]) -> None: + """Bind this handler to one prevalidated DNS result set.""" + super().__init__() + self._addresses = addresses + + def http_open(self, req: urllib.request.Request) -> Any: + """Open an HTTP request without resolving its hostname again.""" + return self.do_open( + lambda host, **kwargs: PinnedHTTPConnection( + host, validated_addresses=self._addresses, **kwargs + ), + req, + ) + + +class PinnedHTTPSHandler(urllib.request.HTTPSHandler): + """urllib handler that pins TCP while preserving HTTPS hostname verification.""" + + def __init__(self, addresses: frozenset[IpAddress]) -> None: + """Bind this handler to one prevalidated DNS result set.""" + super().__init__() + self._addresses = addresses + + def https_open(self, req: urllib.request.Request) -> Any: + """Open HTTPS using the validated address set and original URL hostname.""" + return self.do_open( + lambda host, **kwargs: PinnedHTTPSConnection( + host, validated_addresses=self._addresses, **kwargs + ), + req, + context=self._context, + ) + + +def resolve_endpoint_addresses( + hostname: str, + port: int, +) -> frozenset[IpAddress]: + """Resolve every stream address for an endpoint or fail closed.""" + try: + addrinfo = socket.getaddrinfo(hostname, port, type=socket.SOCK_STREAM) + except (OSError, UnicodeError) as exc: + raise ValueError("Noema endpoint DNS resolution failed") from exc + if not addrinfo: + raise ValueError("Noema endpoint DNS resolution returned no addresses") + + addresses: set[IpAddress] = set() + for result in addrinfo: + try: + raw_address = result[4][0] + address = ipaddress.ip_address(raw_address) + except (IndexError, TypeError, ValueError) as exc: + raise ValueError("Noema endpoint DNS resolution returned a malformed address") from exc + addresses.add(address) + return frozenset(addresses) + + +def validate_endpoint( + api_url: str, +) -> tuple[str, int, frozenset[IpAddress]]: + """Validate transport and pre-request DNS evidence for a model endpoint.""" + if not (api_url.lower().startswith("http://") or api_url.lower().startswith("https://")): + raise ValueError( + "URL scheme must be http or https; NOEMA_LLM_API_URL must start " + "with http:// or https:// to prevent SSRF vulnerabilities" + ) + parsed = urllib.parse.urlparse(api_url) + scheme = parsed.scheme.lower() + if scheme not in {"http", "https"}: + raise ValueError( + "URL scheme must be http or https; NOEMA_LLM_API_URL must start " + "with http:// or https://" + ) + hostname = (parsed.hostname or "").lower() + if not hostname: + raise ValueError("URL must have a valid hostname") + if parsed.username is not None or parsed.password is not None: + raise ValueError("Noema endpoint URL cannot contain user information") + if hostname in {"localhost", "localhost.localdomain"} or hostname.endswith(".localhost"): + raise ValueError("URL cannot target localhost") + + port = parsed.port or (443 if scheme == "https" else 80) + try: + literal_address = ipaddress.ip_address(hostname) + except ValueError: + literal_address = None + trusted_loopback = literal_address in TRUSTED_LOOPBACK_ADDRESSES + if not trusted_loopback and scheme != "https": + raise ValueError("Noema non-loopback endpoints must use HTTPS") + + addresses = resolve_endpoint_addresses(hostname, port) + if trusted_loopback: + if any(not address.is_loopback for address in addresses): + raise ValueError("Noema loopback endpoint DNS resolution left loopback") + elif any( + not address.is_global or address.is_multicast for address in addresses + ): + raise ValueError( + "Noema non-loopback endpoint DNS must contain only globally routable unicast addresses" + ) + return hostname, port, addresses + + def extract_json_object(text: str) -> dict[str, Any]: """Extract a JSON object from a strict or lightly wrapped LLM response.""" stripped = text.strip() @@ -449,32 +621,7 @@ def call_llm( model = os.environ.get("NOEMA_LLM_MODEL", "").strip() or "noema-default" if not api_url or not api_key: raise RuntimeError("Noema LLM review unavailable: NOEMA_LLM_API_URL or NOEMA_LLM_API_KEY is not configured.") - if not (api_url.lower().startswith("http://") or api_url.lower().startswith("https://")): - raise ValueError( - "URL scheme must be http or https; NOEMA_LLM_API_URL must start " - "with http:// or https:// to prevent SSRF vulnerabilities" - ) - parsed = urllib.parse.urlparse(api_url) - if parsed.scheme.lower() not in {"http", "https"}: - raise ValueError("URL scheme must be http or https; NOEMA_LLM_API_URL must start with http:// or https://") - hostname = (parsed.hostname or "").lower() - if not hostname: - raise ValueError("URL must have a valid hostname") - if hostname in {"localhost", "localhost.localdomain"} or hostname.endswith(".localhost"): - raise ValueError("URL cannot target localhost") - try: - addrinfo = socket.getaddrinfo(hostname, None) - except socket.gaierror: - pass - else: - for result in addrinfo: - ip_str = result[4][0] - try: - ip = ipaddress.ip_address(ip_str) - except ValueError: - continue - if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_unspecified: - raise ValueError("URL cannot target internal IP addresses") + hostname, port, addresses = validate_endpoint(api_url) prompt = { "role": "user", @@ -514,9 +661,21 @@ def call_llm( }, method="POST", ) - opener = urllib.request.build_opener(NoRedirectHandler()) + opener = urllib.request.build_opener( + # Force the pinned direct path; proxy destinations have not passed this + # endpoint's DNS policy and must not receive the bearer credential. + urllib.request.ProxyHandler({}), + PinnedHTTPHandler(addresses), + PinnedHTTPSHandler(addresses), + NoRedirectHandler(), + ) with opener.open(request, timeout=120) as response: # nosec B310 - raw = response.read().decode("utf-8") + raw_bytes = response.read(MAX_LLM_RESPONSE_BYTES + 1) + if len(raw_bytes) > MAX_LLM_RESPONSE_BYTES: + raise RuntimeError("Noema LLM response exceeded the byte limit") + if resolve_endpoint_addresses(hostname, port) != addresses: + raise ValueError("Noema endpoint DNS addresses changed during the request") + raw = raw_bytes.decode("utf-8") data = json.loads(raw) content = (((data.get("choices") or [{}])[0].get("message") or {}).get("content") or "").strip() verdict = extract_json_object(content) diff --git a/tests/test_noema_endpoint_boundary.py b/tests/test_noema_endpoint_boundary.py new file mode 100644 index 000000000..d7098111c --- /dev/null +++ b/tests/test_noema_endpoint_boundary.py @@ -0,0 +1,531 @@ +"""Credential-egress contracts for Noema's configurable model endpoint.""" + +from __future__ import annotations + +import json +import socket +from collections.abc import Callable +from typing import Any + +import pytest + +from scripts.ci import noema_review_gate as noema + + +APPROVAL_RESPONSE = { + "choices": [ + { + "message": { + "content": json.dumps( + {"decision": "approve", "summary": "ok", "findings": []} + ) + } + } + ] +} + + +class FakeResponse: + """Return bounded response bytes through urllib's context-manager contract.""" + + def __init__(self, payload: dict[str, Any]) -> None: + """Serialize one deterministic response payload.""" + self.raw = json.dumps(payload).encode("utf-8") + + def __enter__(self) -> FakeResponse: + """Return this response for the request context.""" + return self + + def __exit__(self, *_args: object) -> bool: + """Propagate exceptions raised while consuming the response.""" + return False + + def read(self, size: int = -1) -> bytes: + """Return at most the caller's requested number of bytes.""" + return self.raw if size < 0 else self.raw[:size] + + +class FakeOpener: + """Capture credentialed requests and return a deterministic response.""" + + def __init__( + self, + payload: dict[str, Any], + capture: Callable[[noema.urllib.request.Request], None] | None = None, + ) -> None: + """Store response data and an optional request observer.""" + self.payload = payload + self.capture = capture + + def open( + self, + request: noema.urllib.request.Request, + timeout: int | None = None, + ) -> FakeResponse: + """Record the request and return the configured response.""" + assert timeout == 120 + if self.capture is not None: + self.capture(request) + return FakeResponse(self.payload) + + +def _pr() -> dict[str, Any]: + """Return the minimum current-head pull-request envelope for a model call.""" + return {"title": "review me", "headRefOid": "a" * 40} + + +def _configure(monkeypatch: pytest.MonkeyPatch, url: str) -> None: + """Configure one synthetic Noema model endpoint without a real credential.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", url) + monkeypatch.setenv("NOEMA_LLM_API_KEY", "unit-test-key") + monkeypatch.setenv("NOEMA_LLM_MODEL", "review-model") + + +def _addrinfo(address: str, port: int) -> list[tuple[Any, ...]]: + """Return one getaddrinfo-compatible stream address record.""" + family = socket.AF_INET6 if ":" in address else socket.AF_INET + sockaddr: tuple[Any, ...] = ( + (address, port, 0, 0) if family == socket.AF_INET6 else (address, port) + ) + return [(family, socket.SOCK_STREAM, socket.IPPROTO_TCP, "", sockaddr)] + + +def test_pinned_http_connection_uses_only_validated_numeric_destination( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Prevent a second hostname resolution from selecting an unvalidated peer.""" + fake_socket = object() + observed: list[tuple[tuple[object, ...], float | object, object]] = [] + + def fake_create_connection( + target: tuple[object, ...], + timeout: float | object, + source_address: object, + ) -> object: + observed.append((target, timeout, source_address)) + return fake_socket + + monkeypatch.setattr(noema.socket, "create_connection", fake_create_connection) + connection = noema.PinnedHTTPConnection( + "model.example.test", + port=443, + timeout=17, + validated_addresses=frozenset({noema.ipaddress.ip_address("8.8.8.8")}), + ) + + connection.connect() + + assert connection.sock is fake_socket + assert observed == [(('8.8.8.8', 443), 17, None)] + + +def test_pinned_connection_rejects_empty_validation_evidence() -> None: + """Fail closed when endpoint validation produced no usable address.""" + with pytest.raises(ValueError, match="no validated DNS addresses"): + noema.PinnedHTTPConnection( + "model.example.test", + validated_addresses=frozenset(), + ) + + +def test_pinned_connection_reports_exhausted_validated_addresses( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Report a connection error after every validated address is exhausted.""" + monkeypatch.setattr( + noema.socket, + "create_connection", + lambda *_args: (_ for _ in ()).throw(OSError("unavailable")), + ) + connection = noema.PinnedHTTPConnection( + "model.example.test", + validated_addresses=frozenset({noema.ipaddress.ip_address("8.8.8.8")}), + ) + + with pytest.raises(OSError, match="could not connect to validated DNS addresses"): + connection.connect() + + +def test_pinned_http_connection_preserves_proxy_tunnel( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Connect to the validated address before establishing an HTTP proxy tunnel.""" + monkeypatch.setattr(noema.socket, "create_connection", lambda *_args: object()) + connection = noema.PinnedHTTPConnection( + "model.example.test", + validated_addresses=frozenset({noema.ipaddress.ip_address("8.8.8.8")}), + ) + connection.set_tunnel("proxy.example.test") + tunneled = False + + def fake_tunnel() -> None: + nonlocal tunneled + tunneled = True + + monkeypatch.setattr(connection, "_tunnel", fake_tunnel) + connection.connect() + + assert tunneled + + +def test_pinned_https_connection_preserves_proxy_tunnel_hostname( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Use the proxy hostname for TLS after an HTTPS proxy tunnel is opened.""" + fake_socket = object() + wrapped_socket = object() + + class Context: + """Record the hostname supplied for the tunneled TLS handshake.""" + + def wrap_socket(self, value: object, *, server_hostname: str) -> object: + """Return the wrapped socket after checking the proxy hostname.""" + assert value is fake_socket + assert server_hostname == "proxy.example.test" + return wrapped_socket + + monkeypatch.setattr(noema.socket, "create_connection", lambda *_args: fake_socket) + connection = noema.PinnedHTTPSConnection( + "model.example.test", + context=Context(), + validated_addresses=frozenset({noema.ipaddress.ip_address("8.8.8.8")}), + ) + connection.set_tunnel("proxy.example.test") + monkeypatch.setattr(connection, "_tunnel", lambda: None) + + connection.connect() + + assert connection.sock is wrapped_socket + + +def test_pinned_connection_supports_ipv6_destination_shape( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Pass IPv6 literals to create_connection in its accepted two-field form.""" + fake_socket = object() + observed: list[tuple[object, ...]] = [] + + def fake_create_connection( + target: tuple[object, ...], + _timeout: float | object, + _source_address: object, + ) -> object: + observed.append(target) + return fake_socket + + monkeypatch.setattr(noema.socket, "create_connection", fake_create_connection) + connection = noema.PinnedHTTPConnection( + "model.example.test", + port=443, + validated_addresses=frozenset({noema.ipaddress.ip_address("::1")}), + ) + + connection.connect() + + assert connection.sock is fake_socket + assert observed == [("::1", 443)] + + +def test_pinned_connection_retries_only_other_validated_addresses( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Retry a failed connection only within the already validated address set.""" + attempts: list[tuple[object, ...]] = [] + fake_socket = object() + + def fake_create_connection( + target: tuple[object, ...], + _timeout: float | object, + _source_address: object, + ) -> object: + attempts.append(target) + if len(attempts) == 1: + raise OSError("first validated address unavailable") + return fake_socket + + monkeypatch.setattr(noema.socket, "create_connection", fake_create_connection) + connection = noema.PinnedHTTPConnection( + "model.example.test", + port=443, + validated_addresses=frozenset( + { + noema.ipaddress.ip_address("8.8.8.8"), + noema.ipaddress.ip_address("1.1.1.1"), + } + ), + ) + + connection.connect() + + assert connection.sock is fake_socket + assert attempts == [("1.1.1.1", 443), ("8.8.8.8", 443)] + + +def test_pinned_https_connection_keeps_hostname_for_tls( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Pin TCP to the validated IP while retaining the URL hostname for TLS SNI.""" + fake_socket = object() + wrapped_socket = object() + + class Context: + """Provide the small SSL context surface used by HTTPSConnection.""" + + check_hostname = True + + def wrap_socket(self, value: object, *, server_hostname: str) -> object: + """Record the original hostname and return the wrapped socket.""" + assert value is fake_socket + assert server_hostname == "model.example.test" + return wrapped_socket + + monkeypatch.setattr( + noema.socket, + "create_connection", + lambda *_args: fake_socket, + ) + connection = noema.PinnedHTTPSConnection( + "model.example.test", + port=443, + context=Context(), + validated_addresses=frozenset({noema.ipaddress.ip_address("8.8.8.8")}), + ) + + connection.connect() + + assert connection.sock is wrapped_socket + + +@pytest.mark.parametrize( + ("handler_type", "connection_type"), + [ + (noema.PinnedHTTPHandler, noema.PinnedHTTPConnection), + (noema.PinnedHTTPSHandler, noema.PinnedHTTPSConnection), + ], +) +def test_pinned_handlers_construct_pinned_connections( + monkeypatch: pytest.MonkeyPatch, + handler_type: type[Any], + connection_type: type[Any], +) -> None: + """Keep urllib's HTTP and HTTPS paths on the validated connection classes.""" + addresses = frozenset({noema.ipaddress.ip_address("8.8.8.8")}) + handler = handler_type(addresses) + observed: dict[str, Any] = {} + + def fake_do_open(factory: Any, request: object, **kwargs: Any) -> str: + observed["request"] = request + observed["connection"] = factory("model.example.test", timeout=3, **kwargs) + return "opened" + + monkeypatch.setattr(handler, "do_open", fake_do_open) + request = object() + opened = handler.http_open(request) if isinstance(handler, noema.PinnedHTTPHandler) else handler.https_open(request) + + assert opened == "opened" + assert observed["request"] is request + assert isinstance(observed["connection"], connection_type) + assert observed["connection"]._validated_addresses == tuple(addresses) + + +def test_public_endpoint_requires_https_and_stable_global_dns( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reject plaintext transport and pre/post-request DNS identity drift.""" + _configure(monkeypatch, "http://model.example.test/v1/chat/completions") + monkeypatch.setattr( + socket, + "getaddrinfo", + lambda *_args, **_kwargs: _addrinfo("8.8.8.8", 80), + ) + with pytest.raises(ValueError, match="non-loopback endpoints must use HTTPS"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) + + _configure(monkeypatch, "https://model.example.test/v1/chat/completions") + resolutions = iter( + [_addrinfo("8.8.8.8", 443), _addrinfo("1.1.1.1", 443)] + ) + monkeypatch.setattr(socket, "getaddrinfo", lambda *_args, **_kwargs: next(resolutions)) + sent: list[noema.urllib.request.Request] = [] + monkeypatch.setattr( + noema.urllib.request, + "build_opener", + lambda *_args: FakeOpener(APPROVAL_RESPONSE, sent.append), + ) + + with pytest.raises(ValueError, match="DNS addresses changed during the request"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) + assert len(sent) == 1 + + +def test_public_endpoint_accepts_stable_global_dual_stack_dns( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Accept an HTTPS endpoint only when every A and AAAA result stays global.""" + _configure(monkeypatch, "https://model.example.test/v1/chat/completions") + records = [*_addrinfo("8.8.8.8", 443), *_addrinfo("2606:4700:4700::1111", 443)] + monkeypatch.setattr(socket, "getaddrinfo", lambda *_args, **_kwargs: records) + monkeypatch.setattr( + noema.urllib.request, + "build_opener", + lambda *_args: FakeOpener(APPROVAL_RESPONSE), + ) + + verdict = noema.call_llm("owner/repo", 1, _pr(), "diff", False) + assert verdict["decision"] == "approve" + + +@pytest.mark.parametrize( + ("url", "address"), + [ + ("http://127.0.0.1:43123/v1/chat/completions", "127.0.0.1"), + ("http://[::1]:43123/v1/chat/completions", "::1"), + ], +) +def test_trusted_loopback_sidecar_keeps_the_narrow_http_exception( + monkeypatch: pytest.MonkeyPatch, + url: str, + address: str, +) -> None: + """Allow the existing same-job orchestrator address without opening remote HTTP.""" + _configure(monkeypatch, url) + monkeypatch.setattr( + socket, + "getaddrinfo", + lambda *_args, **_kwargs: _addrinfo(address, 43123), + ) + observed: list[noema.urllib.request.Request] = [] + monkeypatch.setattr( + noema.urllib.request, + "build_opener", + lambda *_args: FakeOpener(APPROVAL_RESPONSE, observed.append), + ) + + verdict = noema.call_llm("owner/repo", 1, _pr(), "diff", False) + assert verdict["decision"] == "approve" + assert observed[0].full_url == url + + +def test_loopback_exception_requires_literal_and_loopback_only_dns( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reject a loopback literal if resolver evidence escapes loopback.""" + _configure(monkeypatch, "http://127.0.0.1:43123/v1/chat/completions") + monkeypatch.setattr( + socket, + "getaddrinfo", + lambda *_args, **_kwargs: _addrinfo("8.8.8.8", 43123), + ) + + with pytest.raises(ValueError, match="left loopback"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) + + +def test_loopback_exception_rejects_other_loopback_literals( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Keep plaintext access narrower than the whole IPv4 loopback block.""" + _configure(monkeypatch, "http://127.0.0.2:43123/v1/chat/completions") + + with pytest.raises(ValueError, match="non-loopback endpoints must use HTTPS"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) + + +def test_endpoint_rejects_url_user_information( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reject embedded URL credentials before resolving or sending the API key.""" + _configure(monkeypatch, "https://user:pass@model.example.test/v1/chat/completions") + + with pytest.raises(ValueError, match="cannot contain user information"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) + + +@pytest.mark.parametrize( + "address", + [ + "0.0.0.0", + "10.0.0.5", + "100.64.0.1", + "127.0.0.1", + "169.254.169.254", + "192.0.2.1", + "224.0.0.1", + "::", + "::1", + "fe80::1", + "ff02::1", + ], +) +def test_non_loopback_hostname_rejects_every_special_address( + monkeypatch: pytest.MonkeyPatch, + address: str, +) -> None: + """Reject private, shared, loopback, link-local, reserved, and multicast DNS.""" + _configure(monkeypatch, "https://model.example.test/v1/chat/completions") + monkeypatch.setattr( + socket, + "getaddrinfo", + lambda *_args, **_kwargs: _addrinfo(address, 443), + ) + + class UnexpectedOpener: + """Fail if invalid DNS evidence reaches a credentialed request.""" + + def open(self, *_args: object, **_kwargs: object) -> None: + """Reject any attempted network call.""" + pytest.fail("credentialed request crossed special-address validation") + + monkeypatch.setattr( + noema.urllib.request, + "build_opener", + lambda *_args: UnexpectedOpener(), + ) + with pytest.raises(ValueError, match="globally routable unicast"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) + + +@pytest.mark.parametrize("failure", ["dns_error", "empty", "malformed"]) +def test_dns_failure_is_closed_before_request_construction( + monkeypatch: pytest.MonkeyPatch, + failure: str, +) -> None: + """Reject resolver errors, empty answers, and malformed socket addresses.""" + _configure(monkeypatch, "https://model.example.test/v1/chat/completions") + + def fail_resolution(*_args: object, **_kwargs: object) -> list[tuple[Any, ...]]: + """Return the selected invalid resolver outcome.""" + if failure == "dns_error": + raise socket.gaierror("unavailable") + if failure == "empty": + return [] + return [(socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP, "", ())] + + monkeypatch.setattr(socket, "getaddrinfo", fail_resolution) + with pytest.raises(ValueError, match="DNS resolution"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) + + +def test_response_body_is_bounded_before_json_decoding( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reject more than one MiB of response data without allocating the full body.""" + _configure(monkeypatch, "https://model.example.test/v1/chat/completions") + monkeypatch.setattr( + socket, + "getaddrinfo", + lambda *_args, **_kwargs: _addrinfo("8.8.8.8", 443), + ) + oversized = { + "choices": [ + {"message": {"content": "x" * noema.MAX_LLM_RESPONSE_BYTES}} + ] + } + monkeypatch.setattr( + noema.urllib.request, + "build_opener", + lambda *_args: FakeOpener(oversized), + ) + + with pytest.raises(RuntimeError, match="response exceeded the byte limit"): + noema.call_llm("owner/repo", 1, _pr(), "diff", False) diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 408bb95b9..c4f0d3c1b 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -1,5 +1,6 @@ import base64 import json +import socket import sys import pytest @@ -324,9 +325,10 @@ def __exit__(self, *args): """Propagate exceptions from the with-statement body.""" return False - def read(self): - """Return the payload as encoded JSON bytes.""" - return json.dumps(self.payload).encode("utf-8") + def read(self, size=-1): + """Return at most the requested number of encoded JSON bytes.""" + payload = json.dumps(self.payload).encode("utf-8") + return payload if size < 0 else payload[:size] def test_call_llm_handles_configuration_and_verdicts(monkeypatch): @@ -344,6 +346,8 @@ def test_call_llm_handles_configuration_and_verdicts(monkeypatch): monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") monkeypatch.setenv("NOEMA_LLM_API_KEY", "secret") monkeypatch.setenv("NOEMA_LLM_MODEL", "review-model") + public_address = [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("8.8.8.8", 443))] + monkeypatch.setattr(socket, "getaddrinfo", lambda *_args, **_kwargs: public_address) seen = {} def fake_urlopen(request, timeout): @@ -397,39 +401,52 @@ def fake_urlopen_defer(request, timeout=None): noema.call_llm("owner/repo", 1, pr, "diff", False) # Test internal IP rejection - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://169.254.169.254/chat") - with pytest.raises(ValueError, match="URL cannot target internal IP addresses"): + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://169.254.169.254/chat") + monkeypatch.setattr( + socket, + "getaddrinfo", + lambda *_args, **_kwargs: [ + ( + socket.AF_INET, + socket.SOCK_STREAM, + 6, + "", + ("169.254.169.254", 443), + ) + ], + ) + with pytest.raises(ValueError, match="globally routable unicast"): noema.call_llm("owner/repo", 1, pr, "diff", False) - import socket original_getaddrinfo = socket.getaddrinfo # Test DNS resolution bypass - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://resolved-to-local.example.com/chat") + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://resolved-to-local.example.com/chat") def fake_getaddrinfo(host, port, *args, **kwargs): if host == "resolved-to-local.example.com": return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("127.0.0.1", 0))] return original_getaddrinfo(host, port, *args, **kwargs) monkeypatch.setattr(socket, "getaddrinfo", fake_getaddrinfo) - with pytest.raises(ValueError, match="URL cannot target internal IP addresses"): + with pytest.raises(ValueError, match="globally routable unicast"): noema.call_llm("owner/repo", 1, pr, "diff", False) - # Test unresolved hostname does not break - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://unresolved.example.com/chat") + # Test unresolved hostname fails closed + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://unresolved.example.com/chat") def fake_getaddrinfo_error(host, port, *args, **kwargs): raise socket.gaierror("Name or service not known") monkeypatch.setattr(socket, "getaddrinfo", fake_getaddrinfo_error) - monkeypatch.setattr(noema.urllib.request, "build_opener", lambda *args: FakeOpener(fake_urlopen)) - assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve" + with pytest.raises(ValueError, match="DNS resolution failed"): + noema.call_llm("owner/repo", 1, pr, "diff", True) - # Test invalid IP string from getaddrinfo (unlikely but theoretically possible) - monkeypatch.setenv("NOEMA_LLM_API_URL", "http://weird-dns.example.com/chat") + # Test invalid IP string from getaddrinfo fails closed + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://weird-dns.example.com/chat") def fake_getaddrinfo_invalid_ip(host, port, *args, **kwargs): if host == "weird-dns.example.com": return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("not_an_ip", 0))] return original_getaddrinfo(host, port, *args, **kwargs) monkeypatch.setattr(socket, "getaddrinfo", fake_getaddrinfo_invalid_ip) - assert noema.call_llm("owner/repo", 1, pr, "diff", True)["decision"] == "approve" + with pytest.raises(ValueError, match="malformed address"): + noema.call_llm("owner/repo", 1, pr, "diff", True) def test_noema_redirect_handler_rejects_redirects(): diff --git a/tests/test_repository_branch_coverage_review_schedulers.py b/tests/test_repository_branch_coverage_review_schedulers.py index d50f94f05..de987b871 100644 --- a/tests/test_repository_branch_coverage_review_schedulers.py +++ b/tests/test_repository_branch_coverage_review_schedulers.py @@ -27,7 +27,7 @@ def test_noema_public_dns_result_reaches_valid_model_response( monkeypatch.setattr( noema.socket, "getaddrinfo", - lambda *_args: [(2, 1, 6, "", ("8.8.8.8", 0))], + lambda *_args, **_kwargs: [(2, 1, 6, "", ("8.8.8.8", 0))], ) class Response: @@ -39,8 +39,8 @@ def __enter__(self) -> "Response": def __exit__(self, *_args: object) -> bool: return False - def read(self) -> bytes: - return json.dumps( + def read(self, size: int = -1) -> bytes: + payload = json.dumps( { "choices": [ { @@ -57,6 +57,7 @@ def read(self) -> bytes: ] } ).encode() + return payload if size < 0 else payload[:size] class Opener: """Open one deterministic provider response."""