fix(server): allow user-owned AI/WebDAV/RPC configs to use private IPs (fixes #244)#246
Conversation
…opback IPs (fixes #244) validateUrl now accepts an allowPrivate option. User-saved config endpoints (AI proxy, WebDAV proxy, RPC download target) use lenient mode so local Ollama / LAN NAS / internal download targets work over http on private addresses. SSRF protection retained: - protocols restricted to http/https - credentials embedded in URL always blocked - cloud metadata IMDS (169.254.169.254) always blocked - IPv4-mapped IPv6 ([::ffff:a.b.c.d]) now normalized and no longer bypasses the filters - semi-trusted inputs (github proxy) stay strict Also makes proxy.test.ts deterministic via vi.mock('axios').
📝 WalkthroughWalkthroughPrivate and loopback URL handling is added to proxy validation. AI, WebDAV, and RPC routes can explicitly permit these destinations, while credentials and the cloud metadata endpoint remain blocked. Proxy tests now mock axios and cover validation, forwarding, responses, and errors. ChangesProxy validation and routing
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an allowPrivate option to the proxy service to permit loopback and private network addresses for user-configured services (such as local AI or WebDAV) while maintaining SSRF protection for cloud metadata IMDS. It also refactors proxy tests to use a mocked axios client for deterministic testing. The review feedback recommends strengthening SSRF protections by normalizing hostnames to strip trailing dots, handling hybrid IPv4-mapped IPv6 addresses, blocking IPv6 private and local ranges in both strict and lenient modes, and adding corresponding unit tests.
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.
| function normalizeHostname(hostname: string): string { | ||
| const h = hostname.replace(/^\[(.+)\]$/, '$1').toLowerCase(); | ||
| const mapped = h.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/); | ||
| if (mapped) { | ||
| const octets = [mapped[1], mapped[2]].flatMap((hex) => { | ||
| const v = parseInt(hex, 16); | ||
| return [(v >>> 8) & 0xff, v & 0xff]; | ||
| }); | ||
| return octets.join('.'); | ||
| } | ||
| return h; | ||
| } |
There was a problem hiding this comment.
To prevent SSRF bypasses, we should normalize hostnames by stripping trailing dots (e.g., localhost. or 127.0.0.1.) and correctly handling hybrid IPv4-mapped IPv6 addresses (e.g., ::ffff:169.254.169.254). Without this, these patterns can bypass the blocked hosts and private IP checks.
function normalizeHostname(hostname: string): string {
// Strip trailing dot if present to prevent SSRF bypasses (e.g., "localhost.")
const h = hostname.replace(/\.$/, '').replace(/^\[(.+)\]$/, '$1').toLowerCase();
// Handle hybrid IPv4-mapped IPv6 (e.g., ::ffff:169.254.169.254)
if (h.startsWith('::ffff:') && h.includes('.')) {
return h.slice(7);
}
const mapped = h.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
if (mapped) {
const octets = [mapped[1], mapped[2]].flatMap((hex) => {
const v = parseInt(hex, 16);
return [(v >>> 8) & 0xff, v & 0xff];
});
return octets.join('.');
}
return h;
}| export function isPrivateOrLoopback(hostname: string): boolean { | ||
| const h = normalizeHostname(hostname); | ||
| if (BLOCKED_HOSTS.has(h)) return true; | ||
| if (PRIVATE_IP_PATTERNS.some(p => p.test(h))) return true; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
We should also block IPv6 private and local ranges (Unique Local Addresses fc00::/7, Link-Local fe80::/10, and Multicast ff00::/8) to prevent SSRF via IPv6.
export function isPrivateOrLoopback(hostname: string): boolean {
const h = normalizeHostname(hostname);
if (BLOCKED_HOSTS.has(h)) return true;
if (PRIVATE_IP_PATTERNS.some(p => p.test(h))) return true;
// Block IPv6 private/local ranges (ULA, Link-Local, Multicast)
if (
h.startsWith('fc') ||
h.startsWith('fd') ||
h.startsWith('fe8') ||
h.startsWith('fe9') ||
h.startsWith('fea') ||
h.startsWith('feb') ||
h.startsWith('ff')
) {
if (h.includes(':')) return true;
}
return false;
}| // 严格模式（默认）：拦截回环地址与私有网段。 | ||
| if (BLOCKED_HOSTS.has(hostname)) { | ||
| throw new Error(`Blocked proxy request: hostname '${hostname}' is not allowed`); | ||
| } | ||
| if (PRIVATE_IP_PATTERNS.some(p => p.test(hostname))) { | ||
| throw new Error(`Blocked proxy request: private IP '${hostname}' is not allowed`); | ||
| } | ||
|
|
||
| // 阻止URL中的用户名和密码（防止凭证泄露） | ||
| if (parsed.username || parsed.password) { | ||
| throw new Error(`Blocked proxy request: URL containing credentials is not allowed`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure that IPv6 private and local ranges are also blocked in strict mode within validateUrl.
// 严格模式（默认）：拦截回环地址与私有网段。
if (BLOCKED_HOSTS.has(hostname)) {
throw new Error(`Blocked proxy request: hostname '${hostname}' is not allowed`);
}
if (PRIVATE_IP_PATTERNS.some(p => p.test(hostname))) {
throw new Error(`Blocked proxy request: private IP '${hostname}' is not allowed`);
}
if (
hostname.startsWith('fc') ||
hostname.startsWith('fd') ||
hostname.startsWith('fe8') ||
hostname.startsWith('fe9') ||
hostname.startsWith('fea') ||
hostname.startsWith('feb') ||
hostname.startsWith('ff')
) {
if (hostname.includes(':')) {
throw new Error(`Blocked proxy request: private IPv6 address '${hostname}' is not allowed`);
}
}
}| it('blocks IPv4-mapped IPv6 forms (e.g. [::ffff:169.254.169.254])', () => { | ||
| expect(() => validateUrl('http://[::ffff:169.254.169.254]/x')).toThrow(/is not allowed/); | ||
| expect(() => validateUrl('http://[::ffff:127.0.0.1]/x')).toThrow(/is not allowed/); | ||
| expect(() => validateUrl('http://[::ffff:192.168.1.10]/x')).toThrow(/private IP/); | ||
| }); |
There was a problem hiding this comment.
Add unit tests to verify that trailing dot bypasses and IPv6 private/local ranges are correctly blocked.
it('blocks IPv4-mapped IPv6 forms (e.g. [::ffff:169.254.169.254])', () => {
expect(() => validateUrl('http://[::ffff:169.254.169.254]/x')).toThrow(/is not allowed/);
expect(() => validateUrl('http://[::ffff:127.0.0.1]/x')).toThrow(/is not allowed/);
expect(() => validateUrl('http://[::ffff:192.168.1.10]/x')).toThrow(/private IP/);
});
it('blocks trailing dot bypasses', () => {
expect(() => validateUrl('http://localhost./x')).toThrow(/is not allowed/);
expect(() => validateUrl('http://127.0.0.1./x')).toThrow(/is not allowed/);
});
it('blocks IPv6 private and local ranges', () => {
expect(() => validateUrl('http://[fd00::1]/x')).toThrow(/private IPv6 address/);
expect(() => validateUrl('http://[fe80::1]/x')).toThrow(/private IPv6 address/);
});![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/routes/proxy.ts (1)
290-300: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict private-network access to saved AI configs Inline requests can still reach private/loopback targets here, so
allowPrivate: trueshould be limited to theconfigIdbranch. That keeps the relaxed SSRF allowance scoped to user-saved configs instead of every API client with the shared secret.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/routes/proxy.ts` around lines 290 - 300, Update the proxyRequest options in the configId handling branch so allowPrivate is true only for saved AI configurations. Remove the unconditional allowPrivate: true from the shared request path, and ensure inline requests retain the default private-network restriction while configId requests keep the relaxed allowance.
🧹 Nitpick comments (1)
server/tests/routes/proxy.test.ts (1)
189-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMock doesn't actually exercise the
ECONNREFUSED-specific axios-error branch.The mocked rejection is a plain
Error('ECONNREFUSED')with noisAxiosError/code, soaxios.isAxiosError(error)returnsfalseinsideproxyRequest, and the assertion is satisfied by the genericBAD_GATEWAYfallback — not the dedicatederror.code === 'ECONNREFUSED'→PROXY_CONNECTION_REFUSEDbranch that a real axios connection-refused error would hit. Worth shaping the mock like a real AxiosError to actually cover that branch.🧪 Proposed fix
- mockAxios.mockRejectedValueOnce(new Error('ECONNREFUSED')); const result = await proxyRequest({ url: 'https://down.example.com/api', method: 'GET', }); - expect(result.status).toBe(502); - expect(result.data).toEqual({ - error: 'Bad Gateway', - code: 'BAD_GATEWAY', - details: 'ECONNREFUSED', - }); + expect(result.status).toBe(502); + expect(result.data).toEqual({ + error: 'Proxy connection refused', + code: 'PROXY_CONNECTION_REFUSED', + details: 'connect ECONNREFUSED', + }); expect(result.headers).toEqual({});And update the rejection above it to:
+ mockAxios.mockRejectedValueOnce({ + isAxiosError: true, + code: 'ECONNREFUSED', + message: 'connect ECONNREFUSED', + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/tests/routes/proxy.test.ts` around lines 189 - 204, Update the mock rejection in the “should return 502 on network error” test to use an Axios-shaped error with isAxiosError true and code ECONNREFUSED, so proxyRequest exercises the dedicated connection-refused branch and asserts its corresponding response code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@server/src/routes/proxy.ts`:
- Around line 290-300: Update the proxyRequest options in the configId handling
branch so allowPrivate is true only for saved AI configurations. Remove the
unconditional allowPrivate: true from the shared request path, and ensure inline
requests retain the default private-network restriction while configId requests
keep the relaxed allowance.
---
Nitpick comments:
In `@server/tests/routes/proxy.test.ts`:
- Around line 189-204: Update the mock rejection in the “should return 502 on
network error” test to use an Axios-shaped error with isAxiosError true and code
ECONNREFUSED, so proxyRequest exercises the dedicated connection-refused branch
and asserts its corresponding response code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aeb0cbc3-b556-4f78-9e75-493eb0815ca3
📒 Files selected for processing (4)
server/src/routes/proxy.tsserver/src/services/proxyService.tsserver/tests/routes/proxy.test.tsserver/tests/services/proxyService.test.ts
- strip trailing dot in hostname to block localhost./127.0.0.1. bypasses - handle hybrid IPv4-mapped IPv6 written with dotted decimals (::ffff:169.254.169.254) - block IPv6 private/local ranges (ULA fc00::/7, link-local fe80::/10, multicast ff00::/8) in both isPrivateOrLoopback and validateUrl strict mode - restrict AI proxy allowPrivate to saved configId branch; inline AI requests keep strict mode to avoid SSRF relaxation abuse - fix proxy.test ECONNREFUSED mock to be axios-shaped so it exercises the dedicated PROXY_CONNECTION_REFUSED branch - add regression tests for trailing-dot and IPv6 private ranges
|
已根据审计意见全部修复并推送到本分支： 安全（阻断绕过，HIGH）
权限收窄（MAJOR）
测试（MEDIUM / 质量）
验证： |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/tests/services/proxyService.test.ts (1)
54-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest names say "private/local" but include multicast
ff02::1.
ff02::1(ff00::/8) is a multicast address, not strictly private or local. The test descriptions at lines 54 and 104 say "IPv6 private/local ranges" which is slightly misleading. Consider updating the test names to include "multicast" for accuracy.✏️ Optional naming fix
- it('blocks IPv6 private/local ranges', () => { + it('blocks IPv6 private/local/multicast ranges', () => {- it('detects IPv6 private/local ranges', () => { + it('detects IPv6 private/local/multicast ranges', () => {Also applies to: 104-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/tests/services/proxyService.test.ts` around lines 54 - 59, Update the test descriptions covering the IPv6 range cases near validateUrl to mention multicast in addition to private/local, accurately reflecting the ff02::1 assertion. Apply the naming change to both affected tests while leaving their assertions and behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/tests/services/proxyService.test.ts`:
- Around line 54-59: Update the test descriptions covering the IPv6 range cases
near validateUrl to mention multicast in addition to private/local, accurately
reflecting the ff02::1 assertion. Apply the naming change to both affected tests
while leaving their assertions and behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5798bb4b-5729-48af-88f6-8324c52c7e72
📒 Files selected for processing (4)
server/src/routes/proxy.tsserver/src/services/proxyService.tsserver/tests/routes/proxy.test.tsserver/tests/services/proxyService.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- server/src/routes/proxy.ts
- server/src/services/proxyService.ts
- server/tests/routes/proxy.test.ts
1 participant
问题
v0.7.0 配置局域网/本机 AI（如 Ollama
http://10.10.16.13或192.168.x.x）时报错：Blocked proxy request: private IP '10.10.16.13' is not allowed。AI API key transmitted over http ...只是无害日志警告，不是失败原因——失败来自validateUrl对私有 IP 的拦截。根因
server/src/services/proxyService.ts的validateUrl一刀切拦截回环/私有网段，被 AI 代理、WebDAV 代理、RPC 下载目标共用。但 AI / WebDAV 的baseUrl是用户自己存进 DB 的服务地址，属可信来源，应与任意/半可信输入区别对待。修复
validateUrl(rawUrl, { allowPrivate })新增宽松档：放行回环/私有网段，仅用于用户自有配置来源的 URL（零额外配置）。169.254.169.254无论如何始终拦截。[::ffff:a.b.c.d]IPv4 映射型 IPv6 原会绕过过滤，现已归一化为对应 IPv4 再校验。proxy.test.ts改为vi.mock('axios')确定性测试（原测试依赖真实外网，环境不通时随机失败）。验证
npx vitest run→ 51/51 通过npx tsc --noEmit→ 0 错误不受影响（确认无需改动）
validateUrl。proxyConfig：作为出口代理传给 axios，非被请求目标。Closes #244
Summary by CodeRabbit