mirror of
https://github.com/nexu-io/open-design.git
synced 2026-05-31 19:04:39 +07:00
fix(security): resolve hostname before approving external API base URLs (#1176)
Some checks failed
ci / Packaged mac smoke (push) Blocked by required conditions
ci / Packaged windows smoke (push) Blocked by required conditions
ci / Detect PR change scopes (push) Failing after 2s
ci / Validate workspace (push) Has been skipped
Docker image / build-and-push (push) Failing after 27s
landing-page-ci / Validate landing page (push) Failing after 1s
landing-page-deploy / Deploy landing page (push) Has been skipped
github-metrics / Generate repository metrics SVG (push) Has been skipped
nix-check / build (push) Failing after 2s
ci / Packaged linux headless smoke (push) Has been skipped
Some checks failed
ci / Packaged mac smoke (push) Blocked by required conditions
ci / Packaged windows smoke (push) Blocked by required conditions
ci / Detect PR change scopes (push) Failing after 2s
ci / Validate workspace (push) Has been skipped
Docker image / build-and-push (push) Failing after 27s
landing-page-ci / Validate landing page (push) Failing after 1s
landing-page-deploy / Deploy landing page (push) Has been skipped
github-metrics / Generate repository metrics SVG (push) Has been skipped
nix-check / build (push) Failing after 2s
ci / Packaged linux headless smoke (push) Has been skipped
* fix(security): resolve hostname before approving external API base URLs
Before this change the daemon-side base-URL validator only inspected the
literal hostname string. A public DNS record that points at an internal
address ('internal.example.com' → 10.0.0.5) passed validation, and the
daemon would issue the upstream request anyway — turning the BYOK proxy
into an SSRF primitive against internal infrastructure.
Add a small companion ('validateBaseUrlResolved') that runs the existing
sync check, resolves the hostname with 'dns.lookup({ all: true })', and
re-applies the block-list against every resolved address. Wire it into
the wrapper the daemon already uses ('validateExternalApiBaseUrl'), so
every proxy/finalize handler picks it up without further edits.
Carve-outs match the existing sync validator:
- Loopback hostnames skip DNS (Ollama-style local LLMs still work,
including '*.localhost' / 'lvh.me'-style names that resolve to 127.0.0.1
per RFC 6761).
- IP literals are already vetted by the sync pass; no need to re-resolve.
- DNS resolver errors fall through to the existing fetch error path —
a transient ENOTFOUND should not turn into a 403.
The 6 callers that previously consumed the sync result now 'await' the
async wrapper. All call sites are already inside async route handlers.
Vitest coverage in apps/daemon/tests/connection-test.test.ts covers:
sync rejection passthrough, loopback / IP-literal short-circuits,
private IPv4 and IPv6 resolution, dual-stack with one private record,
public→public passes (api.openai.com), '*.localhost' resolved→loopback,
and resolver-error fallback.
Detected by: aeon (manual review + semgrep + osv + trufflehog).
Class: CWE-918 (SSRF) — DNS-based bypass.
* fix(security): cover remaining daemon baseUrl fetch surfaces + Ollama redirects
Addresses PR #1176 review feedback (lefarcen / mrcfps): the resolved-IP
wrapper only covered the proxy/finalize routes, leaving three adjacent
SSRF gaps open.
- testProviderConnection (/api/test/connection provider mode): switch
from sync validateBaseUrl to await validateBaseUrlResolved so a
hostname that resolves to a private IP is rejected before the daemon
POSTs the smoke prompt upstream.
- listProviderModels (/api/provider/models): same swap. Import the
DNS-aware helper from ./connectionTest.js since it carries the dns
binding the daemon owns; contracts stays pure.
- Ollama proxy stream fetch: align with the other four proxy routes
by setting redirect: 'error', so a validated public host cannot 3xx
the daemon to a private/internal URL after the pre-fetch check.
Regression coverage:
- POST /api/provider/models — DNS spy returns 10.0.0.5 for a synthetic
hostname; route must respond { ok: false, kind: 'forbidden' } and
must not invoke upstream fetch.
- POST /api/test/connection provider mode — same shape.
- /api/proxy/ollama/stream — fetch mock asserts redirect: 'error' on
the upstream Ollama call.
The existing /api/provider/models timeout test now stubs dnsPromises
so it doesn't race the probe timer against real DNS.
---------
Co-authored-by: aeon <aeon@aaronjmars.com>
This commit is contained in:
parent
6b15236843
commit
9a64fccdc0
7 changed files with 358 additions and 28 deletions
|
|
@ -5,6 +5,7 @@ import {
|
|||
agentIdToTracking,
|
||||
projectKindToTracking,
|
||||
} from '@open-design/contracts/analytics';
|
||||
import { validateBaseUrlResolved } from './connectionTest.js';
|
||||
|
||||
export interface RegisterChatRoutesDeps extends RouteDeps<'db' | 'design' | 'http' | 'chat' | 'agents' | 'critique' | 'validation' | 'lifecycle'> {}
|
||||
|
||||
|
|
@ -46,7 +47,6 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
critiqueResponseCapBytes,
|
||||
critiqueRunRegistry,
|
||||
} = ctx.critique;
|
||||
const { validateBaseUrl } = ctx.validation;
|
||||
const isDaemonShuttingDown = ctx.lifecycle?.isDaemonShuttingDown ?? (() => false);
|
||||
const rejectProxyPluginContext = (body: Record<string, unknown>, res: any) => {
|
||||
if (
|
||||
|
|
@ -507,8 +507,13 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
const redactAuthTokens = (text: string) =>
|
||||
text.replace(/Bearer [A-Za-z0-9_\-.+/=]+/g, 'Bearer [REDACTED]');
|
||||
|
||||
// DNS-aware wrapper. The sync `validateBaseUrl` only inspects the literal
|
||||
// hostname string, so a public DNS name pointing at an internal address
|
||||
// (`internal.example.com → 10.0.0.5`) still passes. We delegate to
|
||||
// `validateBaseUrlResolved` here so every proxy/stream handler runs the
|
||||
// same resolved-IP check before issuing the upstream request.
|
||||
const validateExternalApiBaseUrl = (baseUrl: string) => {
|
||||
return validateBaseUrl(baseUrl);
|
||||
return validateBaseUrlResolved(baseUrl);
|
||||
};
|
||||
|
||||
const proxyErrorCode = (status: number) => {
|
||||
|
|
@ -702,7 +707,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
);
|
||||
}
|
||||
|
||||
const validated = validateExternalApiBaseUrl(baseUrl);
|
||||
const validated = await validateExternalApiBaseUrl(baseUrl);
|
||||
if (validated.error) {
|
||||
return sendApiError(
|
||||
res,
|
||||
|
|
@ -714,7 +719,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
|
||||
const url = appendVersionedApiPath(baseUrl, '/messages');
|
||||
console.log(
|
||||
`[proxy:anthropic] ${req.method} ${validated.parsed.hostname} model=${model}`,
|
||||
`[proxy:anthropic] ${req.method} ${validated.parsed!.hostname} model=${model}`,
|
||||
);
|
||||
|
||||
const payload: any = {
|
||||
|
|
@ -798,7 +803,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
);
|
||||
}
|
||||
|
||||
const validated = validateExternalApiBaseUrl(baseUrl);
|
||||
const validated = await validateExternalApiBaseUrl(baseUrl);
|
||||
if (validated.error) {
|
||||
return sendApiError(
|
||||
res,
|
||||
|
|
@ -810,7 +815,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
|
||||
const url = appendVersionedApiPath(baseUrl, '/chat/completions');
|
||||
console.log(
|
||||
`[proxy:openai] ${req.method} ${validated.parsed.hostname} model=${model}`,
|
||||
`[proxy:openai] ${req.method} ${validated.parsed!.hostname} model=${model}`,
|
||||
);
|
||||
|
||||
const payloadMessages = Array.isArray(messages) ? [...messages] : [];
|
||||
|
|
@ -894,7 +899,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
);
|
||||
}
|
||||
|
||||
const validated = validateExternalApiBaseUrl(baseUrl);
|
||||
const validated = await validateExternalApiBaseUrl(baseUrl);
|
||||
if (validated.error) {
|
||||
return sendApiError(
|
||||
res,
|
||||
|
|
@ -923,7 +928,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
url.searchParams.set('api-version', version);
|
||||
}
|
||||
console.log(
|
||||
`[proxy:azure] ${req.method} ${validated.parsed.hostname} deployment=${model} api-version=${version || 'omitted'}`,
|
||||
`[proxy:azure] ${req.method} ${validated.parsed!.hostname} deployment=${model} api-version=${version || 'omitted'}`,
|
||||
);
|
||||
|
||||
const payloadMessages = Array.isArray(messages) ? [...messages] : [];
|
||||
|
|
@ -1007,7 +1012,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
}
|
||||
|
||||
const effectiveBaseUrl = baseUrl || 'https://generativelanguage.googleapis.com';
|
||||
const validated = validateExternalApiBaseUrl(effectiveBaseUrl);
|
||||
const validated = await validateExternalApiBaseUrl(effectiveBaseUrl);
|
||||
if (validated.error) {
|
||||
return sendApiError(
|
||||
res,
|
||||
|
|
@ -1020,7 +1025,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
const clean = effectiveBaseUrl.replace(/\/+$/, '');
|
||||
const url = `${clean}/v1beta/models/${encodeURIComponent(model)}:streamGenerateContent?alt=sse`;
|
||||
console.log(
|
||||
`[proxy:google] ${req.method} ${validated.parsed.hostname} model=${model}`,
|
||||
`[proxy:google] ${req.method} ${validated.parsed!.hostname} model=${model}`,
|
||||
);
|
||||
|
||||
const contents = (Array.isArray(messages) ? messages : []).map((message) => ({
|
||||
|
|
@ -1101,7 +1106,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
}
|
||||
|
||||
const effectiveBaseUrl = baseUrl || 'https://ollama.com';
|
||||
const validated = validateExternalApiBaseUrl(effectiveBaseUrl);
|
||||
const validated = await validateExternalApiBaseUrl(effectiveBaseUrl);
|
||||
if (validated.error) {
|
||||
return sendApiError(
|
||||
res,
|
||||
|
|
@ -1113,7 +1118,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
|
||||
const clean = effectiveBaseUrl.replace(/\/+$/, '').replace(/\/api\/?$/, '');
|
||||
const url = `${clean}/api/chat`;
|
||||
console.log(`[proxy:ollama] ${req.method} ${validated.parsed.hostname} model=${model}`);
|
||||
console.log(`[proxy:ollama] ${req.method} ${validated.parsed!.hostname} model=${model}`);
|
||||
|
||||
const payloadMessages = Array.isArray(messages) ? [...messages] : [];
|
||||
if (typeof systemPrompt === 'string' && systemPrompt) {
|
||||
|
|
@ -1132,6 +1137,7 @@ export function registerChatRoutes(app: Express, ctx: RegisterChatRoutesDeps) {
|
|||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${apiKey}` },
|
||||
body: JSON.stringify(payload),
|
||||
redirect: 'error',
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@
|
|||
// contracts so Settings and daemon-side checks reject the same hosts.
|
||||
|
||||
import { spawn } from 'node:child_process';
|
||||
import { promises as dnsPromises } from 'node:dns';
|
||||
import { promises as fsp } from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
|
@ -41,9 +42,11 @@ import {
|
|||
} from './runtimes/auth.js';
|
||||
import type { AgentCliEnvPrefs } from './app-config.js';
|
||||
import {
|
||||
isBlockedExternalApiHostname,
|
||||
isLoopbackApiHost,
|
||||
validateBaseUrl,
|
||||
type AgentTestRequest,
|
||||
type BaseUrlValidationResult,
|
||||
type ConnectionTestKind,
|
||||
type ConnectionTestProtocol,
|
||||
type ConnectionTestResponse,
|
||||
|
|
@ -53,6 +56,68 @@ import {
|
|||
|
||||
export { validateBaseUrl } from '@open-design/contracts/api/connectionTest';
|
||||
|
||||
// DNS-aware companion to `validateBaseUrl`. The contracts-side check only
|
||||
// inspects the literal hostname string, so a public DNS name pointing at
|
||||
// internal infrastructure (`internal.example.com → 10.0.0.5`) slips through
|
||||
// and the daemon ends up issuing a request to a private address on behalf of
|
||||
// whichever caller supplied the base URL. Resolve the hostname and re-run
|
||||
// the block-list against every address the system would actually connect to.
|
||||
//
|
||||
// Loopback is intentionally allowed for local LLM providers like Ollama; any
|
||||
// hostname that resolves to a loopback address (including `*.localhost` per
|
||||
// RFC 6761 and IPv4-mapped IPv6 loopback) follows that same carve-out.
|
||||
//
|
||||
// DNS lookup failures are *not* treated as a security signal — the caller is
|
||||
// going to surface a connection error from `fetch` anyway, and turning a
|
||||
// transient resolver hiccup into a 403 would just confuse users. The sync
|
||||
// hostname check still rejected the obvious literal-IP cases before we ever
|
||||
// got here.
|
||||
|
||||
export type DnsLookupAddress = { address: string; family: number };
|
||||
export type DnsLookupFn = (hostname: string) => Promise<DnsLookupAddress[]>;
|
||||
|
||||
const defaultDnsLookup: DnsLookupFn = async (hostname) => {
|
||||
const result = await dnsPromises.lookup(hostname, { all: true, family: 0 });
|
||||
return result.map(({ address, family }) => ({ address, family }));
|
||||
};
|
||||
|
||||
function looksLikeIpLiteral(hostname: string): boolean {
|
||||
const host = hostname.startsWith('[') && hostname.endsWith(']')
|
||||
? hostname.slice(1, -1)
|
||||
: hostname;
|
||||
if (/^\d{1,3}(?:\.\d{1,3}){3}$/.test(host)) return true;
|
||||
return host.includes(':');
|
||||
}
|
||||
|
||||
export async function validateBaseUrlResolved(
|
||||
baseUrl: string,
|
||||
lookup: DnsLookupFn = defaultDnsLookup,
|
||||
): Promise<BaseUrlValidationResult> {
|
||||
const sync = validateBaseUrl(baseUrl);
|
||||
if (sync.error || !sync.parsed) return sync;
|
||||
|
||||
const hostname = sync.parsed.hostname.toLowerCase();
|
||||
if (isLoopbackApiHost(hostname)) return sync;
|
||||
if (looksLikeIpLiteral(hostname)) return sync;
|
||||
|
||||
let addresses: DnsLookupAddress[];
|
||||
try {
|
||||
addresses = await lookup(hostname);
|
||||
} catch {
|
||||
return sync;
|
||||
}
|
||||
|
||||
for (const addr of addresses) {
|
||||
const ip = String(addr.address).toLowerCase();
|
||||
if (isLoopbackApiHost(ip)) continue;
|
||||
if (isBlockedExternalApiHostname(ip)) {
|
||||
return { error: 'Internal IPs blocked', forbidden: true };
|
||||
}
|
||||
}
|
||||
|
||||
return sync;
|
||||
}
|
||||
|
||||
// Aggressive but not punitive — happy paths usually return in under 2 s.
|
||||
// Override with OD_CONNECTION_TEST_PROVIDER_TIMEOUT_MS for slow networks
|
||||
// or distant providers; invalid values fall back to the default.
|
||||
|
|
@ -557,7 +622,7 @@ export async function testProviderConnection(
|
|||
): Promise<ConnectionTestResponse> {
|
||||
const start = Date.now();
|
||||
const model = String(input.model ?? '');
|
||||
const validated = validateBaseUrl(input.baseUrl);
|
||||
const validated = await validateBaseUrlResolved(input.baseUrl);
|
||||
if (validated.error || !validated.parsed) {
|
||||
const kind: ConnectionTestKind = validated.forbidden ? 'forbidden' : 'invalid_base_url';
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -558,7 +558,7 @@ export function registerFinalizeRoutes(app: Express, ctx: RegisterFinalizeRoutes
|
|||
if (typeof baseUrl !== 'string' || !baseUrl.trim()) {
|
||||
return sendApiError(res, 400, 'BAD_REQUEST', 'baseUrl must be a non-empty string when provided');
|
||||
}
|
||||
const validated = validateExternalApiBaseUrl(baseUrl);
|
||||
const validated = await validateExternalApiBaseUrl(baseUrl);
|
||||
if (validated.error) {
|
||||
return sendApiError(
|
||||
res,
|
||||
|
|
|
|||
|
|
@ -7,8 +7,8 @@ import type {
|
|||
ProviderModelsRequest,
|
||||
ProviderModelsResponse,
|
||||
} from '@open-design/contracts/api/providerModels';
|
||||
import { isLoopbackApiHost, validateBaseUrl } from '@open-design/contracts/api/connectionTest';
|
||||
import { redactSecrets } from './connectionTest.js';
|
||||
import { isLoopbackApiHost } from '@open-design/contracts/api/connectionTest';
|
||||
import { redactSecrets, validateBaseUrlResolved } from './connectionTest.js';
|
||||
|
||||
type ProviderModelsInput = ProviderModelsRequest & { signal?: AbortSignal };
|
||||
|
||||
|
|
@ -197,7 +197,7 @@ export async function listProviderModels(
|
|||
};
|
||||
}
|
||||
|
||||
const validated = validateBaseUrl(input.baseUrl);
|
||||
const validated = await validateBaseUrlResolved(input.baseUrl);
|
||||
if (validated.error || !validated.parsed) {
|
||||
return {
|
||||
ok: false,
|
||||
|
|
|
|||
|
|
@ -145,6 +145,7 @@ import {
|
|||
testAgentConnection,
|
||||
testProviderConnection,
|
||||
validateBaseUrl,
|
||||
validateBaseUrlResolved,
|
||||
} from './connectionTest.js';
|
||||
import { listProviderModels } from './providerModels.js';
|
||||
import { importClaudeDesignZip } from './claude-design-import.js';
|
||||
|
|
@ -3487,7 +3488,12 @@ export async function startServer({
|
|||
getAppVersion: () => cachedAppVersion,
|
||||
});
|
||||
|
||||
const validateExternalApiBaseUrl = (baseUrl) => validateBaseUrl(baseUrl);
|
||||
// DNS-aware wrapper. The sync `validateBaseUrl` only inspects the literal
|
||||
// hostname string, so a public DNS name pointing at an internal address
|
||||
// (`internal.example.com → 10.0.0.5`) still passes. We delegate to
|
||||
// `validateBaseUrlResolved` here so every proxy and finalize handler runs
|
||||
// the same resolved-IP check before issuing the upstream request.
|
||||
const validateExternalApiBaseUrl = (baseUrl) => validateBaseUrlResolved(baseUrl);
|
||||
|
||||
const resolvedPortRef = {
|
||||
get current() {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
// provider protocol and uses fake CLI bins for deterministic agent outcomes.
|
||||
|
||||
import type http from 'node:http';
|
||||
import { promises as dnsPromises } from 'node:dns';
|
||||
import { promises as fsp } from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
|
@ -13,6 +14,8 @@ import {
|
|||
resolveConnectionTestTimeoutMs,
|
||||
testAgentConnection,
|
||||
testProviderConnection,
|
||||
validateBaseUrlResolved,
|
||||
type DnsLookupAddress,
|
||||
} from '../src/connectionTest.js';
|
||||
import { listProviderModels } from '../src/providerModels.js';
|
||||
import { startServer } from '../src/server.js';
|
||||
|
|
@ -358,7 +361,53 @@ describe('POST /api/provider/models', () => {
|
|||
).toBe(false);
|
||||
});
|
||||
|
||||
// Regression for the DNS-bypass SSRF gap flagged on PR #1176: the route
|
||||
// must resolve the hostname and reject when *any* resolved address is in
|
||||
// a blocked range, not just when the literal hostname is a private IP.
|
||||
it('rejects hostnames that resolve to a private IP without calling upstream fetch', async () => {
|
||||
const fetchMock = passThroughOrUpstream(() => jsonResponse({}));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const dnsSpy = vi
|
||||
.spyOn(dnsPromises, 'lookup')
|
||||
.mockImplementation((async (hostname: string) => {
|
||||
if (hostname === 'rebind.example.test') {
|
||||
return [{ address: '10.0.0.5', family: 4 }];
|
||||
}
|
||||
const err: NodeJS.ErrnoException = new Error('ENOTFOUND');
|
||||
err.code = 'ENOTFOUND';
|
||||
throw err;
|
||||
}) as unknown as typeof dnsPromises.lookup);
|
||||
try {
|
||||
const res = await realFetch(`${baseUrl}/api/provider/models`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
protocol: 'openai',
|
||||
baseUrl: 'https://rebind.example.test/v1',
|
||||
apiKey: 'sk-good',
|
||||
}),
|
||||
});
|
||||
const body = (await res.json()) as Record<string, unknown>;
|
||||
expect(body).toMatchObject({ ok: false, kind: 'forbidden' });
|
||||
expect(
|
||||
fetchMock.mock.calls.some(
|
||||
([input]) => !String(input).startsWith(baseUrl),
|
||||
),
|
||||
).toBe(false);
|
||||
} finally {
|
||||
dnsSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('reports timeout when model listing is aborted by the probe timer', async () => {
|
||||
// The DNS-aware validator runs before the probe timer is installed; stub
|
||||
// the resolver so the test doesn't race against real DNS while fake
|
||||
// timers are active.
|
||||
const dnsSpy = vi
|
||||
.spyOn(dnsPromises, 'lookup')
|
||||
.mockImplementation((async () => [
|
||||
{ address: '203.0.113.10', family: 4 },
|
||||
]) as unknown as typeof dnsPromises.lookup);
|
||||
vi.useFakeTimers();
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
|
|
@ -371,17 +420,21 @@ describe('POST /api/provider/models', () => {
|
|||
),
|
||||
);
|
||||
|
||||
const pending = listProviderModels({
|
||||
protocol: 'openai',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
apiKey: 'sk-timeout',
|
||||
});
|
||||
try {
|
||||
const pending = listProviderModels({
|
||||
protocol: 'openai',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
apiKey: 'sk-timeout',
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(12_000);
|
||||
await expect(pending).resolves.toMatchObject({
|
||||
ok: false,
|
||||
kind: 'timeout',
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(12_000);
|
||||
await expect(pending).resolves.toMatchObject({
|
||||
ok: false,
|
||||
kind: 'timeout',
|
||||
});
|
||||
} finally {
|
||||
dnsSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -771,6 +824,47 @@ describe('POST /api/test/connection provider mode', () => {
|
|||
).toBe(false);
|
||||
});
|
||||
|
||||
// Regression for the DNS-bypass SSRF gap flagged on PR #1176: provider
|
||||
// mode must run the same resolved-IP check as the proxy/finalize paths
|
||||
// so a public hostname pointing at a private address can't be fetched.
|
||||
it('reports forbidden for hostnames that resolve to a private IP without calling fetch', async () => {
|
||||
const fetchMock = passThroughOrUpstream(() => jsonResponse({}));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const dnsSpy = vi
|
||||
.spyOn(dnsPromises, 'lookup')
|
||||
.mockImplementation((async (hostname: string) => {
|
||||
if (hostname === 'rebind.example.test') {
|
||||
return [{ address: '10.0.0.5', family: 4 }];
|
||||
}
|
||||
const err: NodeJS.ErrnoException = new Error('ENOTFOUND');
|
||||
err.code = 'ENOTFOUND';
|
||||
throw err;
|
||||
}) as unknown as typeof dnsPromises.lookup);
|
||||
try {
|
||||
const res = await realFetch(`${baseUrl}/api/test/connection`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
mode: 'provider',
|
||||
protocol: 'openai',
|
||||
baseUrl: 'https://rebind.example.test/v1',
|
||||
apiKey: 'sk-good',
|
||||
model: 'gpt-4o',
|
||||
}),
|
||||
});
|
||||
const body = (await res.json()) as Record<string, unknown>;
|
||||
expect(body.ok).toBe(false);
|
||||
expect(body.kind).toBe('forbidden');
|
||||
expect(
|
||||
fetchMock.mock.calls.some(
|
||||
([input]) => !String(input).startsWith(baseUrl),
|
||||
),
|
||||
).toBe(false);
|
||||
} finally {
|
||||
dnsSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('allows IPv6 loopback base URLs for local OpenAI-compatible providers', async () => {
|
||||
for (const loopbackBaseUrl of [
|
||||
'http://[::1]:1234/v1',
|
||||
|
|
@ -1945,3 +2039,127 @@ describe('connection test timeout overrides', () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateBaseUrlResolved (DNS-aware base URL validation)', () => {
|
||||
function lookupReturning(addresses: DnsLookupAddress[]) {
|
||||
return vi.fn(async () => addresses);
|
||||
}
|
||||
|
||||
it('passes through the contracts-level error for invalid input', async () => {
|
||||
expect(await validateBaseUrlResolved('not-a-url', lookupReturning([]))).toMatchObject({
|
||||
error: 'Invalid baseUrl',
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects the literal-IP cases the sync check already catches', async () => {
|
||||
for (const baseUrl of [
|
||||
'http://10.0.0.5:11434/v1',
|
||||
'http://169.254.169.254/latest/meta-data',
|
||||
'http://[fd00::1]:11434/v1',
|
||||
'http://[fe80::1]:11434/v1',
|
||||
]) {
|
||||
expect(await validateBaseUrlResolved(baseUrl, lookupReturning([]))).toMatchObject({
|
||||
error: 'Internal IPs blocked',
|
||||
forbidden: true,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('skips DNS for loopback hostnames so local LLMs (Ollama, *.localhost) still work', async () => {
|
||||
const lookup = lookupReturning([{ address: '127.0.0.1', family: 4 }]);
|
||||
for (const baseUrl of [
|
||||
'http://localhost:11434/v1',
|
||||
'http://127.0.0.1:11434/v1',
|
||||
'http://[::1]:11434/v1',
|
||||
]) {
|
||||
const result = await validateBaseUrlResolved(baseUrl, lookup);
|
||||
expect(result.error).toBeUndefined();
|
||||
}
|
||||
expect(lookup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips DNS for IP-literal hostnames the sync check already vetted', async () => {
|
||||
const lookup = lookupReturning([]);
|
||||
expect((await validateBaseUrlResolved('https://1.2.3.4/v1', lookup)).error).toBeUndefined();
|
||||
expect((await validateBaseUrlResolved('https://[2606:4700::]/v1', lookup)).error).toBeUndefined();
|
||||
expect(lookup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects public hostnames that resolve to private IPv4 ranges', async () => {
|
||||
const cases: Array<{ resolved: string; family: number }> = [
|
||||
{ resolved: '10.0.0.5', family: 4 },
|
||||
{ resolved: '172.16.0.5', family: 4 },
|
||||
{ resolved: '192.168.1.5', family: 4 },
|
||||
{ resolved: '100.64.0.1', family: 4 },
|
||||
{ resolved: '169.254.169.254', family: 4 },
|
||||
{ resolved: '0.0.0.0', family: 4 },
|
||||
{ resolved: '224.0.0.1', family: 4 },
|
||||
];
|
||||
for (const { resolved, family } of cases) {
|
||||
const result = await validateBaseUrlResolved(
|
||||
'https://internal.example.com/v1',
|
||||
lookupReturning([{ address: resolved, family }]),
|
||||
);
|
||||
expect(result).toMatchObject({
|
||||
error: 'Internal IPs blocked',
|
||||
forbidden: true,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects public hostnames that resolve to private IPv6 ranges', async () => {
|
||||
for (const resolved of ['fd00::1', 'fe80::1', '::']) {
|
||||
const result = await validateBaseUrlResolved(
|
||||
'https://internal.example.com/v1',
|
||||
lookupReturning([{ address: resolved, family: 6 }]),
|
||||
);
|
||||
expect(result).toMatchObject({
|
||||
error: 'Internal IPs blocked',
|
||||
forbidden: true,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects when ANY resolved record (round-robin / dual-stack) is internal', async () => {
|
||||
const result = await validateBaseUrlResolved(
|
||||
'https://mixed.example.com/v1',
|
||||
lookupReturning([
|
||||
{ address: '52.84.10.1', family: 4 },
|
||||
{ address: '10.0.0.5', family: 4 },
|
||||
]),
|
||||
);
|
||||
expect(result).toMatchObject({
|
||||
error: 'Internal IPs blocked',
|
||||
forbidden: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('allows public hostnames that resolve to public addresses (the api.openai.com case)', async () => {
|
||||
const result = await validateBaseUrlResolved(
|
||||
'https://api.openai.com/v1',
|
||||
lookupReturning([
|
||||
{ address: '104.18.7.192', family: 4 },
|
||||
{ address: '2606:4700::6812:7c0', family: 6 },
|
||||
]),
|
||||
);
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(result.parsed?.hostname).toBe('api.openai.com');
|
||||
});
|
||||
|
||||
it('allows hostnames that resolve to loopback (e.g. *.localhost / lvh.me)', async () => {
|
||||
const result = await validateBaseUrlResolved(
|
||||
'http://app.localhost:11434/v1',
|
||||
lookupReturning([{ address: '127.0.0.1', family: 4 }]),
|
||||
);
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back to allow-through on DNS resolver errors so transient failures are not 403s', async () => {
|
||||
const failingLookup = vi.fn(async () => {
|
||||
throw new Error('ENOTFOUND');
|
||||
});
|
||||
const result = await validateBaseUrlResolved('https://offline.example.com/v1', failingLookup);
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(failingLookup).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -488,6 +488,41 @@ describe('API proxy routes', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// Regression for PR #1176: the Ollama proxy fetch must also set
|
||||
// `redirect: 'error'`. Without it, a validated public host could
|
||||
// 3xx the daemon to a private/internal URL and slip past the
|
||||
// resolved-IP SSRF check that runs *before* the fetch.
|
||||
it('forwards redirect:error on the Ollama proxy upstream fetch', async () => {
|
||||
const ndjsonResponse = new Response(
|
||||
new TextEncoder().encode('{"done":true}\n'),
|
||||
{
|
||||
status: 200,
|
||||
headers: { 'content-type': 'application/x-ndjson' },
|
||||
},
|
||||
);
|
||||
const fetchMock = vi.fn((input: FetchInput, init?: FetchInit) => {
|
||||
const url = String(input);
|
||||
if (url.startsWith(baseUrl)) return realFetch(input, init);
|
||||
return Promise.resolve(ndjsonResponse);
|
||||
});
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
await realFetch(`${baseUrl}/api/proxy/ollama/stream`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
baseUrl: 'https://ollama.example.com',
|
||||
apiKey: 'ollama-key',
|
||||
model: 'llama3',
|
||||
messages: [{ role: 'user', content: 'hello' }],
|
||||
}),
|
||||
});
|
||||
|
||||
const [upstreamUrl, upstreamInit] = fetchMock.mock.calls[0]!;
|
||||
expect(String(upstreamUrl)).toBe('https://ollama.example.com/api/chat');
|
||||
expect(upstreamInit?.redirect).toBe('error');
|
||||
});
|
||||
|
||||
// Plan §3.A4 / spec §11.8 (e2e-7): the API-fallback proxy paths must
|
||||
// never carry plugin context. The web sidecar's fallback mode bypasses
|
||||
// the daemon snapshot bus, so any pluginId / appliedPluginSnapshotId in
|
||||
|
|
|
|||
Loading…
Reference in a new issue