mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
feat(net): the SSRF-guarded requests go through HTTP_PROXY / HTTPS_PROXY too
Place photos, link previews, webhooks and the integrations fetch through the guard's own dispatcher and so ignored the outbound proxy, which left a server that can only reach the internet through one without them. They now read HTTP_PROXY, HTTPS_PROXY and NO_PROXY themselves and connect through the proxy, after the guard has vetted the target. The Helm chart passes the three variables through. (#1754)
This commit is contained in:
Maurice
@@ -70,6 +70,15 @@ data:
|
||||
{{- if .Values.env.OIDC_SCOPE }}
|
||||
OIDC_SCOPE: {{ .Values.env.OIDC_SCOPE | quote }}
|
||||
{{- end }}
|
||||
{{- if .Values.env.HTTP_PROXY }}
|
||||
HTTP_PROXY: {{ .Values.env.HTTP_PROXY | quote }}
|
||||
{{- end }}
|
||||
{{- if .Values.env.HTTPS_PROXY }}
|
||||
HTTPS_PROXY: {{ .Values.env.HTTPS_PROXY | quote }}
|
||||
{{- end }}
|
||||
{{- if .Values.env.NO_PROXY }}
|
||||
NO_PROXY: {{ .Values.env.NO_PROXY | quote }}
|
||||
{{- end }}
|
||||
{{- if .Values.env.OIDC_DISCOVERY_URL }}
|
||||
OIDC_DISCOVERY_URL: {{ .Values.env.OIDC_DISCOVERY_URL | quote }}
|
||||
{{- end }}
|
||||
|
||||
@@ -77,6 +77,10 @@ env:
|
||||
# Space-separated OIDC scopes to request. Must include scopes for any claim used by OIDC_ADMIN_CLAIM.
|
||||
# OIDC_DISCOVERY_URL: ""
|
||||
# Override the OIDC discovery endpoint for providers with non-standard paths (e.g. Authentik).
|
||||
# HTTP_PROXY: ""
|
||||
# HTTPS_PROXY: ""
|
||||
# NO_PROXY: "localhost,127.0.0.1"
|
||||
# Outbound proxy for TREK's requests to the internet; NO_PROXY lists hosts that bypass it.
|
||||
# DEMO_MODE: "false"
|
||||
# Enable demo mode (hourly data resets).
|
||||
# MCP_RATE_LIMIT: "300"
|
||||
|
||||
@@ -343,6 +343,16 @@ export function deriveNet(raw: RawEnv) {
|
||||
return {
|
||||
allowInternalNetwork: parseBool(raw.ALLOW_INTERNAL_NETWORK) === true,
|
||||
allowLinkLocalIps: parseLinkLocalAllowList(raw.ALLOW_LINK_LOCAL_IPS).ips,
|
||||
// The variables Node's own env proxy reads, both spellings, so the guarded
|
||||
// requests below follow the same proxy as everything else (#1754).
|
||||
proxy: {
|
||||
http: (raw.HTTP_PROXY ?? raw.http_proxy)?.trim() || undefined,
|
||||
https: (raw.HTTPS_PROXY ?? raw.https_proxy)?.trim() || undefined,
|
||||
noProxy: ((raw.NO_PROXY ?? raw.no_proxy) || '')
|
||||
.split(',')
|
||||
.map(entry => entry.trim().toLowerCase())
|
||||
.filter(Boolean),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@ import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
import { avatarUrl } from '../common/avatarUrl';
|
||||
import { checkSsrf, createPinnedDispatcher } from '../../utils/ssrfGuard';
|
||||
import { checkSsrf, createOutboundDispatcher } from '../../utils/ssrfGuard';
|
||||
import { discardBody, exceedsDeclaredLength, readCappedText } from '../../utils/cappedFetch';
|
||||
import type { CollabNote, CollabPoll, CollabMessage, TripFile, User } from '../../types';
|
||||
import { NotificationsService } from '../notifications/notifications.service';
|
||||
@@ -662,7 +662,7 @@ export class CollabService {
|
||||
return { ...fallback, error: 'URL not allowed' };
|
||||
}
|
||||
|
||||
const dispatcher = createPinnedDispatcher(ssrf.resolvedIp!);
|
||||
const dispatcher = createOutboundDispatcher(url, ssrf.resolvedIp!);
|
||||
try {
|
||||
// AbortSignal.timeout covers the body as well. The hand-rolled controller
|
||||
// this replaces was cleared as soon as the headers arrived, so a server that
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import dns from 'node:dns/promises';
|
||||
import { Agent } from 'undici';
|
||||
import { Agent, ProxyAgent, type Dispatcher } from 'undici';
|
||||
import { readEnv } from '../app-config';
|
||||
import { embeddedTransitionIpv4, expandIpv6 } from './ipv6';
|
||||
|
||||
@@ -9,6 +9,8 @@ const ALLOW_INTERNAL_NETWORK = readEnv().net.allowInternalNetwork;
|
||||
// host gateway being the reason (#2400). The parser never lets a cloud metadata
|
||||
// address in, so the rest of 169.254.0.0/16 stays blocked. Frozen the same way.
|
||||
const ALLOWED_LINK_LOCAL = new Set(readEnv().net.allowLinkLocalIps);
|
||||
// HTTP_PROXY / HTTPS_PROXY / NO_PROXY (#1754). Frozen like the two above.
|
||||
const PROXY = readEnv().net.proxy;
|
||||
|
||||
export interface SsrfResult {
|
||||
allowed: boolean;
|
||||
@@ -291,7 +293,7 @@ export async function safeFetchAdminConfigured(
|
||||
throw new SsrfBlockedError('Requests to link-local / cloud-metadata addresses are not allowed');
|
||||
}
|
||||
|
||||
const dispatcher = createPinnedDispatcher(usableIps, true, responseTimeoutMs);
|
||||
const dispatcher = createOutboundDispatcher(currentUrl, usableIps, true, responseTimeoutMs);
|
||||
const response = await fetch(currentUrl, { ...hopInit, redirect: 'manual', dispatcher } as any);
|
||||
|
||||
// Only a 3xx WITH a Location header is a redirect we follow; anything else
|
||||
@@ -485,7 +487,7 @@ export async function safeFetchFollow(
|
||||
throw new SsrfBlockedError(ssrf.error ?? 'Request blocked by SSRF guard');
|
||||
}
|
||||
|
||||
const dispatcher = createPinnedDispatcher(ssrf.resolvedIps ?? [ssrf.resolvedIp!], rejectUnauthorized);
|
||||
const dispatcher = createOutboundDispatcher(currentUrl, ssrf.resolvedIps ?? [ssrf.resolvedIp!], rejectUnauthorized);
|
||||
const response = await fetch(currentUrl, {
|
||||
...hopInit,
|
||||
redirect: 'manual',
|
||||
@@ -520,6 +522,55 @@ export async function safeFetchFollow(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The proxy a guarded request to this URL goes through, or null (#1754).
|
||||
*
|
||||
* The guarded fetches below use their own dispatcher, so they never saw the
|
||||
* environment proxy Node applies to everything else, and on a server that can
|
||||
* only reach the internet through a proxy every place photo, link preview and
|
||||
* webhook simply failed. NO_PROXY works the way curl and Node read it: `*`
|
||||
* matches every host, an entry matches the host itself and its subdomains
|
||||
* (a leading dot is optional), and `host:port` narrows it to one port.
|
||||
*/
|
||||
export function proxyFor(rawUrl: string, proxy = PROXY): string | null {
|
||||
let url: URL;
|
||||
try { url = new URL(rawUrl); } catch { return null; }
|
||||
const target = url.protocol === 'https:' ? proxy.https ?? proxy.http : proxy.http;
|
||||
if (!target) return null;
|
||||
const host = url.hostname.toLowerCase().replace(/^\[|\]$/g, '');
|
||||
const port = url.port || (url.protocol === 'https:' ? '443' : '80');
|
||||
const bypass = proxy.noProxy.some((entry) => {
|
||||
if (entry === '*') return true;
|
||||
const [name, entryPort] = entry.replace(/^\[|\](?=:|$)/g, '').split(/:(?=\d+$)/);
|
||||
if (entryPort && entryPort !== port) return false;
|
||||
const bare = name.replace(/^\*?\./, '');
|
||||
return host === bare || host.endsWith(`.${bare}`);
|
||||
});
|
||||
return bypass ? null : target;
|
||||
}
|
||||
|
||||
/**
|
||||
* The dispatcher one guarded hop goes out through. Without a proxy it is the
|
||||
* pinned agent below. With one, the proxy makes the connection, so the address
|
||||
* cannot be pinned on our side: the guard has already refused a private or
|
||||
* loopback target before this point, and the proxy the operator chose is the
|
||||
* network boundary from there on.
|
||||
*/
|
||||
export function createOutboundDispatcher(
|
||||
url: string,
|
||||
resolved: string | readonly string[],
|
||||
rejectUnauthorized = true,
|
||||
responseTimeoutMs?: number,
|
||||
): Dispatcher {
|
||||
const proxy = proxyFor(url);
|
||||
if (!proxy) return createPinnedDispatcher(resolved, rejectUnauthorized, responseTimeoutMs);
|
||||
return new ProxyAgent({
|
||||
uri: proxy,
|
||||
requestTls: { rejectUnauthorized },
|
||||
...(responseTimeoutMs ? { headersTimeout: responseTimeoutMs, bodyTimeout: responseTimeoutMs } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns an undici Agent whose connect.lookup is pinned to the already-validated
|
||||
* addresses. This prevents DNS rebinding (TOCTOU) by ensuring the outbound
|
||||
|
||||
@@ -61,7 +61,7 @@ vi.mock('../../../src/utils/ssrfGuard', () => {
|
||||
}
|
||||
return {
|
||||
checkSsrf: mockCheckSsrf,
|
||||
createPinnedDispatcher: mockCreatePinnedDispatcher,
|
||||
createOutboundDispatcher: mockCreatePinnedDispatcher,
|
||||
SsrfBlockedError,
|
||||
// The notification transports go through safeFetchFollow now, so the fake
|
||||
// has to guard and then hand over to the stubbed fetch the way it does.
|
||||
@@ -545,7 +545,7 @@ describe('linkPreview hardening', () => {
|
||||
// the pin does not cover that hop: Node skips the pinned lookup for a literal IP.
|
||||
expect(init.redirect).toBe('error');
|
||||
expect(init.signal).toBeInstanceOf(AbortSignal);
|
||||
expect(mockCreatePinnedDispatcher).toHaveBeenCalledWith('93.184.216.34');
|
||||
expect(mockCreatePinnedDispatcher).toHaveBeenCalledWith('https://example.com/init', '93.184.216.34');
|
||||
expect(init.dispatcher).toBe(dispatcher);
|
||||
// One Agent is built per preview; leaving it open leaks its sockets.
|
||||
expect(dispatcher.close).toHaveBeenCalled();
|
||||
|
||||
@@ -24,7 +24,7 @@ vi.mock('dns/promises', () => ({ default: { lookup: vi.fn() }, lookup: vi.fn() }
|
||||
|
||||
// ssrfGuard reads env at module load, so the mock must answer before the import.
|
||||
const { readEnvMock } = vi.hoisted(() => ({
|
||||
readEnvMock: vi.fn(() => ({ net: { allowInternalNetwork: true }, integrations: { llmTimeoutMs: 900_000 } })),
|
||||
readEnvMock: vi.fn(() => ({ net: { allowInternalNetwork: true, proxy: { noProxy: [] } }, integrations: { llmTimeoutMs: 900_000 } })),
|
||||
}));
|
||||
vi.mock('../../../src/app-config', () => ({ readEnv: readEnvMock }));
|
||||
|
||||
@@ -36,7 +36,7 @@ const mockLookup = vi.mocked(dns.lookup);
|
||||
beforeEach(() => {
|
||||
AgentMock.mockClear();
|
||||
readEnvMock.mockClear();
|
||||
readEnvMock.mockReturnValue({ net: { allowInternalNetwork: true }, integrations: { llmTimeoutMs: 900_000 } });
|
||||
readEnvMock.mockReturnValue({ net: { allowInternalNetwork: true, proxy: { noProxy: [] } }, integrations: { llmTimeoutMs: 900_000 } });
|
||||
mockLookup.mockResolvedValue({ address: '203.0.113.10', family: 4 });
|
||||
});
|
||||
|
||||
@@ -82,7 +82,7 @@ describe('createPinnedDispatcher — response ceiling', () => {
|
||||
|
||||
describe('the ceiling belongs to the model lane only', () => {
|
||||
it('safeFetchLlm carries the configured ceiling to the dispatcher', async () => {
|
||||
readEnvMock.mockReturnValue({ net: { allowInternalNetwork: true }, integrations: { llmTimeoutMs: 120_000 } });
|
||||
readEnvMock.mockReturnValue({ net: { allowInternalNetwork: true, proxy: { noProxy: [] } }, integrations: { llmTimeoutMs: 120_000 } });
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValue(response({ status: 200 })));
|
||||
|
||||
await safeFetchLlm('https://api.provider.example/v1/chat/completions');
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* Guarded requests and the outbound proxy (#1754). The SSRF guard fetches through
|
||||
* its own dispatcher, so it used to skip HTTP_PROXY entirely; these pin when it
|
||||
* goes through the proxy and when NO_PROXY keeps it direct.
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const built = vi.hoisted(() => ({ agents: [] as unknown[], proxies: [] as Array<Record<string, unknown>> }));
|
||||
|
||||
vi.mock('undici', () => ({
|
||||
Agent: class { constructor(opts: unknown) { built.agents.push(opts); } },
|
||||
ProxyAgent: class { constructor(opts: Record<string, unknown>) { built.proxies.push(opts); } },
|
||||
}));
|
||||
|
||||
const PROXY_KEYS = ['HTTP_PROXY', 'HTTPS_PROXY', 'NO_PROXY', 'http_proxy', 'https_proxy', 'no_proxy'] as const;
|
||||
|
||||
async function loadGuard(env: Partial<Record<(typeof PROXY_KEYS)[number], string>>) {
|
||||
for (const key of PROXY_KEYS) delete process.env[key];
|
||||
Object.assign(process.env, env);
|
||||
vi.resetModules();
|
||||
return import('../../../src/utils/ssrfGuard');
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const key of PROXY_KEYS) delete process.env[key];
|
||||
built.agents.length = 0;
|
||||
built.proxies.length = 0;
|
||||
});
|
||||
|
||||
describe('proxyFor', () => {
|
||||
it('SSRF-PROXY-001: no proxy configured means a direct connection', async () => {
|
||||
const { proxyFor } = await loadGuard({});
|
||||
expect(proxyFor('https://example.com/a')).toBeNull();
|
||||
});
|
||||
|
||||
it('SSRF-PROXY-002: picks the proxy by scheme, HTTPS falling back to HTTP_PROXY', async () => {
|
||||
const { proxyFor } = await loadGuard({ HTTP_PROXY: 'http://plain:3128' });
|
||||
expect(proxyFor('http://example.com/')).toBe('http://plain:3128');
|
||||
expect(proxyFor('https://example.com/')).toBe('http://plain:3128');
|
||||
const both = await loadGuard({ HTTP_PROXY: 'http://plain:3128', https_proxy: 'http://secure:3129' });
|
||||
expect(both.proxyFor('https://example.com/')).toBe('http://secure:3129');
|
||||
});
|
||||
|
||||
it('SSRF-PROXY-003: NO_PROXY matches the host, its subdomains, a port and the wildcard', async () => {
|
||||
const { proxyFor } = await loadGuard({
|
||||
HTTPS_PROXY: 'http://proxy:3128',
|
||||
NO_PROXY: 'localhost, .internal.lan, photos.example.com:8443',
|
||||
});
|
||||
expect(proxyFor('https://localhost/health')).toBeNull();
|
||||
expect(proxyFor('https://immich.internal.lan/api')).toBeNull();
|
||||
expect(proxyFor('https://internal.lan/')).toBeNull();
|
||||
expect(proxyFor('https://photos.example.com:8443/x')).toBeNull();
|
||||
// Same host on another port, and a lookalike domain, still go through the proxy.
|
||||
expect(proxyFor('https://photos.example.com/x')).toBe('http://proxy:3128');
|
||||
expect(proxyFor('https://notinternal.lan/')).toBe('http://proxy:3128');
|
||||
|
||||
const all = await loadGuard({ HTTPS_PROXY: 'http://proxy:3128', NO_PROXY: '*' });
|
||||
expect(all.proxyFor('https://example.com/')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('createOutboundDispatcher', () => {
|
||||
it('SSRF-PROXY-004: without a proxy it is the pinned agent', async () => {
|
||||
const { createOutboundDispatcher } = await loadGuard({});
|
||||
createOutboundDispatcher('https://example.com/', ['93.184.216.34']);
|
||||
expect(built.agents).toHaveLength(1);
|
||||
expect(built.proxies).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('SSRF-PROXY-005: with a proxy it goes through it, keeping TLS checks and the timeout', async () => {
|
||||
const { createOutboundDispatcher } = await loadGuard({ HTTPS_PROXY: 'http://proxy:3128' });
|
||||
createOutboundDispatcher('https://example.com/', ['93.184.216.34'], false, 30_000);
|
||||
expect(built.agents).toHaveLength(0);
|
||||
expect(built.proxies[0]).toEqual({
|
||||
uri: 'http://proxy:3128',
|
||||
requestTls: { rejectUnauthorized: false },
|
||||
headersTimeout: 30_000,
|
||||
bodyTimeout: 30_000,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -145,13 +145,15 @@ proxying is disabled by default.
|
||||
| `HTTPS_PROXY` | Proxy URL for outbound HTTPS requests | — |
|
||||
| `NO_PROXY` | Comma-separated hosts or domains that should bypass the proxy | — |
|
||||
|
||||
> **Note:** Proxy environment variables apply to requests made through Node.js's default HTTP dispatcher. Requests
|
||||
> handled by TREK's SSRF protection use a dedicated dispatcher and do not use the environment proxy.
|
||||
> **Note:** Requests that go through TREK's SSRF protection (place photos, link previews, webhooks, Immich, Synology,
|
||||
> Dawarich and the other integrations) read the same three variables themselves and use the proxy as well, on any
|
||||
> install. Everything else follows Node's own environment proxy, which needs `NODE_USE_ENV_PROXY=1` (see below).
|
||||
> `NO_PROXY` takes `*`, a host (which also matches its subdomains, a leading dot is optional) or `host:port`.
|
||||
|
||||
> **Container only.** Node ignores these variables unless it is started with `NODE_USE_ENV_PROXY=1`, and the official
|
||||
> image sets that for you. On a source or Proxmox install, set `NODE_USE_ENV_PROXY=1` alongside them or nothing will
|
||||
> change. On Helm the image already has it, but the chart's ConfigMap does not declare `HTTP_PROXY`, `HTTPS_PROXY` or
|
||||
> `NO_PROXY`, so a value under `env:` is dropped; patch the three onto the Deployment instead.
|
||||
> change. On Helm the image already has it, and the chart passes `HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY` from
|
||||
> `env:` through to the container.
|
||||
|
||||
> **Set `NO_PROXY`.** Without it every request goes to the proxy, including the ones TREK makes to itself, such as the
|
||||
> container health check. `localhost,127.0.0.1` is a sensible minimum; add your own hosts as needed.
|
||||
|
||||
Reference in New Issue
Block a user