fix(proxy): ensure AstrBot ignores system proxy variables when no proxy is configured to prevent local API requests from being intercepted - #8897
Conversation
- 在未配置代理时检测系统 http_proxy/https_proxy 变量并输出警告,提示用户正确配置 - 清除可能残留的代理环境变量,避免干扰 AstrBot 内部网络请求 - 强制设置 no_proxy 为 localhost,127.0.0.1,::1,确保 Dashboard 等本地 API 不会被代理错误拦截 - 更新对应单元测试,断言清除后 no_proxy 被正确赋值为本地回环地址 closes AstrBotDevs#8871
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider renaming
hasSystemProxytohas_system_proxyto keep variable naming consistent with Python’s snake_case style used elsewhere in the codebase. - Since you’re deleting and then (re)setting related proxy environment variables together, you could simplify the cleanup with a small helper or loop (e.g., iterating over
['http_proxy', 'https_proxy', 'no_proxy']) to reduce repetition and make future updates to the list easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider renaming `hasSystemProxy` to `has_system_proxy` to keep variable naming consistent with Python’s snake_case style used elsewhere in the codebase.
- Since you’re deleting and then (re)setting related proxy environment variables together, you could simplify the cleanup with a small helper or loop (e.g., iterating over `['http_proxy', 'https_proxy', 'no_proxy']`) to reduce repetition and make future updates to the list easier.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request updates the core lifecycle to warn users when system proxy environment variables are detected without an AstrBot proxy configuration, and ensures local loopback addresses are set in 'no_proxy' to prevent local API requests from being intercepted. The review feedback suggests handling both uppercase and lowercase proxy environment variables (due to case sensitivity in Unix/Linux), adhering to PEP 8 naming conventions for variables, simplifying redundant environment variable operations, and updating unit tests to cover these improvements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| hasSystemProxy = "https_proxy" in os.environ or "http_proxy" in os.environ | ||
| if hasSystemProxy: | ||
| logger.warning( | ||
| "检测到系统环境变量中存在 http_proxy/https_proxy," | ||
| "但 AstrBot 未配置代理。正在清除代理环境变量," | ||
| "并将 no_proxy 设为 localhost,127.0.0.1 以避免本地 API 请求被代理拦截。" | ||
| "如需使用代理,请在 AstrBot 配置中设置 http_proxy。" | ||
| ) | ||
| if "https_proxy" in os.environ: | ||
| del os.environ["https_proxy"] | ||
| if "http_proxy" in os.environ: | ||
| del os.environ["http_proxy"] | ||
| if "no_proxy" in os.environ: | ||
| del os.environ["no_proxy"] | ||
| logger.debug("HTTP proxy cleared") | ||
| # 始终确保本地回环地址不被代理,防止 Dashboard 等本地 API 请求失败 | ||
| os.environ["no_proxy"] = "localhost,127.0.0.1,::1" | ||
| logger.debug("HTTP proxy cleared, no_proxy set to localhost") |
There was a problem hiding this comment.
💡 改进建议:统一处理大小写代理环境变量、遵循 PEP 8 命名规范并简化冗余逻辑
- 大小写敏感性问题:在 Unix/Linux 系统中,环境变量是大小写敏感的。许多 HTTP 客户端库(如
httpx、requests、aiohttp等)会同时读取大写(HTTP_PROXY/HTTPS_PROXY/NO_PROXY)和小写(http_proxy/https_proxy/no_proxy)的环境变量。如果用户系统设置了大写的环境变量,当前的检测和清除逻辑将失效,导致本地请求仍被拦截。 - PEP 8 命名规范:变量名
hasSystemProxy违反了 PEP 8 的snake_case命名规范,应改为has_system_proxy。 - 冗余逻辑:在第 87-88 行中删除
no_proxy之后,又在第 90 行重新赋值,这部分删除操作是多余的。可以使用os.environ.pop(key, None)统一且安全地清除环境变量。
建议将此段逻辑重构为更健壮、更符合规范的实现。
| hasSystemProxy = "https_proxy" in os.environ or "http_proxy" in os.environ | |
| if hasSystemProxy: | |
| logger.warning( | |
| "检测到系统环境变量中存在 http_proxy/https_proxy," | |
| "但 AstrBot 未配置代理。正在清除代理环境变量," | |
| "并将 no_proxy 设为 localhost,127.0.0.1 以避免本地 API 请求被代理拦截。" | |
| "如需使用代理,请在 AstrBot 配置中设置 http_proxy。" | |
| ) | |
| if "https_proxy" in os.environ: | |
| del os.environ["https_proxy"] | |
| if "http_proxy" in os.environ: | |
| del os.environ["http_proxy"] | |
| if "no_proxy" in os.environ: | |
| del os.environ["no_proxy"] | |
| logger.debug("HTTP proxy cleared") | |
| # 始终确保本地回环地址不被代理,防止 Dashboard 等本地 API 请求失败 | |
| os.environ["no_proxy"] = "localhost,127.0.0.1,::1" | |
| logger.debug("HTTP proxy cleared, no_proxy set to localhost") | |
| has_system_proxy = any( | |
| k in os.environ | |
| for k in ("http_proxy", "https_proxy", "HTTP_PROXY", "HTTPS_PROXY") | |
| ) | |
| if has_system_proxy: | |
| logger.warning( | |
| "检测到系统环境变量中存在 http_proxy/https_proxy," | |
| "但 AstrBot 未配置代理。正在清除代理环境变量," | |
| "并将 no_proxy 设为 localhost,127.0.0.1 以避免本地 API 请求被代理拦截。" | |
| "如需使用代理,请在 AstrBot 配置中设置 http_proxy。" | |
| ) | |
| for key in ("http_proxy", "https_proxy", "HTTP_PROXY", "HTTPS_PROXY", "no_proxy", "NO_PROXY"): | |
| os.environ.pop(key, None) | |
| # 始终确保本地回环地址不被代理,防止 Dashboard 等本地 API 请求失败 | |
| os.environ["no_proxy"] = "localhost,127.0.0.1,::1" | |
| os.environ["NO_PROXY"] = "localhost,127.0.0.1,::1" | |
| logger.debug("HTTP proxy cleared, no_proxy set to localhost") |
References
- PEP 8: Variable names should be lowercase, with words separated by underscores as necessary to improve readability (snake_case). (link)
| assert "http_proxy" not in os.environ | ||
| assert "https_proxy" not in os.environ | ||
| # 清除后 no_proxy 应被设为本地回环地址,防止本地 API 被代理拦截 | ||
| assert os.environ.get("no_proxy") == "localhost,127.0.0.1,::1" |
There was a problem hiding this comment.
🧪 单元测试改进建议
为了配合对大写代理环境变量(HTTP_PROXY / HTTPS_PROXY / NO_PROXY)的清除与设置逻辑,建议在单元测试中也增加对应的断言,以确保大写环境变量被正确处理。
| assert "http_proxy" not in os.environ | |
| assert "https_proxy" not in os.environ | |
| # 清除后 no_proxy 应被设为本地回环地址,防止本地 API 被代理拦截 | |
| assert os.environ.get("no_proxy") == "localhost,127.0.0.1,::1" | |
| assert "http_proxy" not in os.environ | |
| assert "https_proxy" not in os.environ | |
| assert "HTTP_PROXY" not in os.environ | |
| assert "HTTPS_PROXY" not in os.environ | |
| # 清除后 no_proxy 应被设为本地回环地址,防止本地 API 被代理拦截 | |
| assert os.environ.get("no_proxy") == "localhost,127.0.0.1,::1" | |
| assert os.environ.get("NO_PROXY") == "localhost,127.0.0.1,::1" |
Modifications / 改动点
closes #8871
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Ensure AstrBot ignores system proxy variables when no proxy is configured to prevent local API requests from being intercepted.
Bug Fixes:
Tests: