fix(proxy): strip unsupported IPv6 CIDRs from NO_PROXY#1999
fix(proxy): strip unsupported IPv6 CIDRs from NO_PROXY#1999pi-dal wants to merge 1 commit intoMoonshotAI:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a startup crash in environments where NO_PROXY includes IPv6 CIDR blocks by normalizing NO_PROXY/no_proxy before httpx client initialization.
Changes:
- Add normalization to strip IPv6 CIDR entries from
NO_PROXY/no_proxywhile keeping non-CIDR entries intact. - Add a regression test that reproduces the
httpx.Client()initialization failure scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/kimi_cli/utils/proxy.py |
Filters unsupported IPv6 CIDR entries from NO_PROXY/no_proxy during proxy env normalization. |
tests/utils/test_proxy.py |
Adds a regression test to ensure httpx.Client() initialization doesn’t crash with IPv6 CIDRs in NO_PROXY. |
Comments suppressed due to low confidence (1)
src/kimi_cli/utils/proxy.py:41
- The
normalize_proxy_env()docstring still only documents thesocks://->socks5://rewrite, but the function now also mutatesNO_PROXY/no_proxyby stripping unsupported IPv6 CIDR entries. Please update the docstring to describe this additional normalization (and the rationale) so callers understand the side effects.
def normalize_proxy_env() -> None:
"""Rewrite ``socks://`` to ``socks5://`` in proxy environment variables.
Many proxy tools (V2RayN, Clash, etc.) set ``ALL_PROXY=socks://...``, but
httpx and aiohttp only recognise ``socks5://``. Since ``socks://`` is
effectively an alias for ``socks5://``, this function performs a safe
in-place replacement so that downstream HTTP clients work correctly.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| monkeypatch.setenv("NO_PROXY", "localhost,127.0.0.1,::1,fd00::/8,fe80::/10") | ||
|
|
||
| normalize_proxy_env() | ||
|
|
There was a problem hiding this comment.
This regression test currently only asserts that httpx.Client() construction succeeds, which makes it less direct about what normalize_proxy_env() is expected to do. Consider also asserting that NO_PROXY is rewritten to remove the IPv6 CIDRs while preserving localhost,127.0.0.1,::1 so the test continues to validate the normalization even if httpx behavior changes in the future.
| assert os.environ["NO_PROXY"] == "localhost,127.0.0.1,::1" |
| client = httpx.Client() | ||
| client.close() |
There was a problem hiding this comment.
Prefer using with httpx.Client() as client: (or contextlib.closing) instead of manual client = ...; client.close() so the client is always closed even if an assertion is added later and fails mid-test.
| client = httpx.Client() | |
| client.close() | |
| with httpx.Client() as client: | |
| pass |
Fixes #1959
Summary
NO_PROXY/no_proxybeforehttpxclient initializationlocalhost,127.0.0.1, and::1Verification
uv run pytest tests/utils/test_proxy.pyuv run ruff check src/kimi_cli/utils/proxy.py tests/utils/test_proxy.pyHTTP_PROXY=http://127.0.0.1:7890andNO_PROXY=localhost,127.0.0.1,::1,fd00::/8,fe80::/10;httpx.Client()now initializes successfully after normalizationmake testFull Check Notes
make ai-testcurrently fails on the existingtests_ai/scripts/main.yamlreference tokimi_cli.tools.multiagent:Task, which is unrelated to this changemake checkstill reports existing non-blockingtydiagnostics and fails on pre-existing web Biome lint errors inweb/src/features/chat/components/approval-dialog.tsxandweb/src/features/chat/components/session-files-panel.tsx