feat: add HTTP(S) proxy support via undici ProxyAgent#37
feat: add HTTP(S) proxy support via undici ProxyAgent#37neurostream wants to merge 5 commits intobrave:mainfrom
Conversation
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
Thank you for the PR, @neurostream. I will be taking a look at this ASAP. |
|
Tested this pr locally, it works pretty well, for those who need proxy to access brave, it is very useful. |
There was a problem hiding this comment.
Pull Request Overview
Adds basic HTTP(S) proxy support using undici’s ProxyAgent and introduces NO_PROXY handling to bypass the proxy for specific hosts.
- Set a global undici ProxyAgent when HTTP(S)_PROXY is present.
- Implement NO_PROXY pattern matching to bypass the proxy on a per-request basis.
- Add undici as a dependency.
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/index.ts | Initialize undici ProxyAgent from env and set as global dispatcher. |
| src/BraveAPI/index.ts | Add NO_PROXY handling and adjust fetch invocation accordingly. |
| package.json | Add undici dependency. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import { setGlobalDispatcher, ProxyAgent } from 'undici'; | ||
| const proxy = process.env.HTTPS_PROXY || process.env.HTTP_PROXY; | ||
| if (proxy) { | ||
| setGlobalDispatcher(new ProxyAgent({ uri: proxy })); |
There was a problem hiding this comment.
ProxyAgent's constructor expects the proxy URL as the first argument (string or URL), not an options object with a uri property. Use new ProxyAgent(proxy) so the agent is created correctly.
| setGlobalDispatcher(new ProxyAgent({ uri: proxy })); | |
| setGlobalDispatcher(new ProxyAgent(proxy)); |
| @@ -1,4 +1,9 @@ | |||
| #!/usr/bin/env node | |||
| import { setGlobalDispatcher, ProxyAgent } from 'undici'; | |||
| const proxy = process.env.HTTPS_PROXY || process.env.HTTP_PROXY; | |||
There was a problem hiding this comment.
[nitpick] Consider supporting lowercase variants (https_proxy/http_proxy) which are commonly used across environments and tooling for better compatibility.
| const proxy = process.env.HTTPS_PROXY || process.env.HTTP_PROXY; | |
| const proxy = | |
| process.env.HTTPS_PROXY || | |
| process.env.https_proxy || | |
| process.env.HTTP_PROXY || | |
| process.env.http_proxy; |
| } | ||
| } | ||
|
|
||
| const response = await fetch(urlWithParams, { headers, ...(agent !== undefined ? { agent } : {}) }); |
There was a problem hiding this comment.
undici's fetch does not support an agent option; per-request overrides must use the dispatcher option. To bypass a globally configured ProxyAgent when NO_PROXY matches, create a plain undici Agent and pass it as dispatcher (e.g., import { Agent } from 'undici'; then use fetch(url, { headers, dispatcher: new Agent() })). Passing agent: null will be ignored and won't bypass the proxy.
| let agent; | ||
| const noProxy = process.env.NO_PROXY || process.env.no_proxy; | ||
| if (noProxy) { | ||
| const { hostname } = parseUrl(urlWithParams); |
There was a problem hiding this comment.
url.parse from node:url is legacy; prefer the WHATWG URL API for hostname extraction: new URL(urlWithParams).hostname. This also allows you to drop the node:url import.
|
@neurostream mind rebasing? |
|
Hi, is there any update on this PR? |
|
Hi, thanks for all the work! I noticed this PR is stalled for some reason. I’d like to help by submitting a refreshed version, and I wonder would that be welcome? |
|
Hi , thanks for the great work , any update on this PR ? it will be merged or not ? |
This PR adds support for basic web proxy configuration, allowing the process to read/use standard environment variables (HTTP_PROXY, HTTPS_PROXY).
Resolves#36.
I tested this in an stdio MCP runtime environment (used codex cli to load and run ghcr.io/neurostream/brave-search-mcp-server:enable-proxy from this branch) to verify the proxy access logs show the api client connecting through:
Thanks for making and packaging this api client for MCP!