-
Notifications
You must be signed in to change notification settings - Fork 80
feat: add HTTP CONNECT proxy support (HTTPS_PROXY) #3912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Users behind corporate proxies/firewalls can now connect to Modal's API by setting MODAL_GRPC_PROXY, HTTPS_PROXY, or https_proxy environment variables. The proxy connection uses HTTP CONNECT tunneling through a ProxyChannel subclass of grpclib's Channel. Env var priority: MODAL_GRPC_PROXY > TOML config > HTTPS_PROXY > https_proxy Co-Authored-By: Claude Opus 4.5 <[email protected]>
PR SummaryMedium Risk Overview Introduces a new Written by Cursor Bugbot for commit 7b27b2d. This will update automatically on new commits. Configure here. |
- Use self._loop instead of asyncio.get_event_loop() (consistency with grpclib base) - Respect self._config.ssl_target_name_override for server_hostname - Validate raw proxy URL for CRLF before urlparse (header injection prevention) - Reject non-http proxy URL schemes (e.g. socks5://) - Add bounded response buffer (16 KiB) and 30s timeout on all socket ops - Decode status line as ASCII with error replacement - Fix socket ownership: set to None after transport takes it, suppress OSError on close - Strip whitespace from HTTPS_PROXY/https_proxy env var fallback - Remove redundant `import socket as _socket` - Fix docstring: "HTTPS gRPC connections" (not "all") - Add tests for scheme validation, CRLF rejection, whitespace env var handling Co-Authored-By: Claude Opus 4.5 <[email protected]>
…vements Refactor _create_connection into two methods with a single outer asyncio.wait_for timeout, add IPv6 proxy resolution via getaddrinfo, percent-decode proxy credentials, add NO_PROXY/no_proxy bypass support, and improve test reliability by eliminating TOCTOU port races and ensuring server cleanup via try/finally. Adds 6 new test cases. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Raise OSError instead of modal ConnectionError from ProxyChannel so get_or_create_channel's except OSError handler wraps proxy failures into user-friendly messages consistently - Wrap DNS resolution inside connect timeout (extract _resolve_and_connect) so hanging resolvers don't exceed _CONNECT_TIMEOUT - Make server_hostname conditional on SSL to prevent ValueError when ssl=None - Reject missing proxy hostname instead of defaulting to localhost - Add host whitespace/tab validation and port range (1..65535) check - Move sock.setblocking(False) inside try block to prevent socket leak - Add __repr__ to ProxyChannel that masks auth credentials - Redact proxy URL credentials in modal config show - Update NO_PROXY docstring to match actual behavior (always honored) - Add tests: DNS timeout, getaddrinfo failure, IPv6 proxy URL, input validation, credential redaction, __repr__ masking, monkeypatch timeout Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
- Prevent double-bracketing of already-bracketed IPv6 hosts in CONNECT request (e.g., [2001:db8::1] no longer becomes [[2001:db8::1]]) - Catch asyncio.TimeoutError alongside TimeoutError for Python 3.10 compatibility (asyncio.TimeoutError became alias of TimeoutError in 3.11) - Preserve IPv6 brackets in _redact_url_credentials output so redacted URLs like http://***@[::1]:3128 remain valid - Add tests: IPv6 double-bracket prevention, IPv6 redaction preservation Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
ProxyChannelsubclass ofgrpclib.client.Channelthat tunnels gRPC connections through an HTTP CONNECT proxygrpc_proxyconfig setting (MODAL_GRPC_PROXYenv var) with fallback to standardHTTPS_PROXY/https_proxyenvironment variablesMotivation
Modal's Python client uses grpclib (pure-Python async gRPC) which calls
asyncio.loop.create_connection()directly — with zero HTTP proxy support. Users behind corporate proxies or firewalls cannot connect toapi.modal.com:443.Approach
grpclib's
Channelhas no custom connection factory parameter.ProxyChanneloverrides_create_connection()to:loop.create_connection(ssl=..., sock=...)for TLS + H2Configuration priority
MODAL_GRPC_PROXY> TOMLgrpc_proxy>HTTPS_PROXY>https_proxy> NoneFiles changed
modal/config.pygrpc_proxysetting +HTTPS_PROXYenv var fallback + docstringmodal/_utils/grpc_utils.pyProxyChannelclass, modifiedcreate_channel()to use ittest/proxy_channel_test.pyTest plan
MODAL_GRPC_PROXY>HTTPS_PROXY>https_proxy> None (4 tests)ProxyChannelfor https+proxy, standardChannelfor http/unix/no-proxy (4 tests)ProxyChannelconstruction: URL parsing, auth encoding, default port (3 tests)🤖 Generated with Claude Code