From fe8832538aca1d454aa90f6da150847b6d6ed384 Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Tue, 28 Jul 2026 17:46:54 +0200 Subject: [PATCH] refactor: use standard undici env based proxy Signed-off-by: William Phetsinorath Change-Id: I080adfe48d5b7f914fe9d92d83f5bc536a6a6964 --- apps/server-nestjs/src/main.ts | 3 + .../open-cds-client.service.spec.ts | 84 +++++++------------ .../service-chain/open-cds-client.service.ts | 21 ++--- apps/server/src/runtime.ts | 23 +++-- 4 files changed, 52 insertions(+), 79 deletions(-) diff --git a/apps/server-nestjs/src/main.ts b/apps/server-nestjs/src/main.ts index 28b5180f5e..c50b7cdae9 100644 --- a/apps/server-nestjs/src/main.ts +++ b/apps/server-nestjs/src/main.ts @@ -8,6 +8,7 @@ import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto' import { PeriodicExportingMetricReader } from '@opentelemetry/sdk-metrics' import { NodeSDK, resources } from '@opentelemetry/sdk-node' import { Logger } from 'nestjs-pino' +import { EnvHttpProxyAgent, setGlobalDispatcher } from 'undici' import { MainModule } from './main.module' import { ConfigurationService } from './modules/infrastructure/configuration/configuration.service' @@ -30,6 +31,8 @@ const telemetry = new NodeSDK({ }) async function bootstrap() { + setGlobalDispatcher(new EnvHttpProxyAgent()) + telemetry.start() const app = await NestFactory.create(MainModule, new FastifyAdapter(), { diff --git a/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.spec.ts b/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.spec.ts index 26b0cfe6af..d0ff874edc 100644 --- a/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.spec.ts +++ b/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.spec.ts @@ -1,8 +1,8 @@ import type { TestingModule } from '@nestjs/testing' -import type { Dispatcher, RequestInit } from 'undici' +import type { RequestInit } from 'undici' import { HttpStatus } from '@nestjs/common' import { Test } from '@nestjs/testing' -import { Agent, fetch, Headers, ProxyAgent, Response } from 'undici' +import { Agent, fetch, Headers, Response } from 'undici' import { beforeEach, describe, expect, it, vi } from 'vitest' import { ConfigurationService } from '../infrastructure/configuration/configuration.service' import { OpenCdsClientError, OpenCdsClientService } from './open-cds-client.service' @@ -14,7 +14,6 @@ vi.mock('undici', async (importOriginal) => { ...actual, Agent: vi.fn(), fetch: vi.fn(), - ProxyAgent: vi.fn(), } }) @@ -31,27 +30,11 @@ describe('openCdsClientService', () => { let module: TestingModule let service: OpenCdsClientService let config: Partial - let tlsDispatcher: Pick - let proxyDispatcher: Pick beforeEach(async () => { vi.clearAllMocks() vi.unstubAllEnvs() - tlsDispatcher = { dispatch: vi.fn() } - proxyDispatcher = { dispatch: vi.fn() } - - class MockAgent { - dispatch = tlsDispatcher - }; - - class ProxyMockAgent { - dispatch = proxyDispatcher - }; - - vi.mocked(Agent).mockImplementation(MockAgent as any) - vi.mocked(ProxyAgent).mockImplementation(ProxyMockAgent as any) - config = { openCdsUrl: 'https://opencds.example.com/root/api/', openCdsApiToken: 'test-token', @@ -68,7 +51,7 @@ describe('openCdsClientService', () => { service = module.get(OpenCdsClientService) }) - it('builds GET requests with an Axios-compatible URL, API key header and TLS-aware dispatcher', async () => { + it('builds GET requests with an Axios-compatible URL, API key header and global dispatcher', async () => { mockFetchResponse(new Response(JSON.stringify({ ok: true }), { status: HttpStatus.OK, headers: { @@ -78,45 +61,15 @@ describe('openCdsClientService', () => { const result = await service.get<{ ok: boolean }>('/requests') - expect(Agent).toHaveBeenCalledWith({ - connect: { - rejectUnauthorized: true, - }, - }) const [url, init] = getLastFetchCall() expect(url).toBe('https://opencds.example.com/root/api/requests') - expect(init.dispatcher?.dispatch).toBe(tlsDispatcher) + expect(init.dispatcher).toBeUndefined() expect(init.method).toBe('GET') expect(init.signal).toBeUndefined() expect(new Headers(init.headers).get('X-API-Key')).toBe('test-token') expect(result).toEqual({ ok: true }) }) - it('uses ProxyAgent when HTTP_PROXY is configured and preserves TLS settings for the upstream request', async () => { - vi.stubEnv('HTTP_PROXY', 'http://proxy.internal:3128') - mockFetchResponse(new Response(JSON.stringify({ ok: true }), { - status: HttpStatus.OK, - headers: { - 'content-type': 'application/json', - }, - })) - - await service.get('/requests') - - expect(ProxyAgent).toHaveBeenCalledWith({ - requestTls: { - rejectUnauthorized: true, - }, - uri: 'http://proxy.internal:3128', - }) - const [url, init] = getLastFetchCall() - expect(url).toBe('https://opencds.example.com/root/api/requests') - expect(init.dispatcher?.dispatch).toBe(proxyDispatcher) - expect(init.method).toBe('GET') - expect(init.signal).toBeUndefined() - expect(new Headers(init.headers).get('X-API-Key')).toBe('test-token') - }) - it('applies query parameters and omits undefined values on GET', async () => { mockFetchResponse(new Response(JSON.stringify({ ok: true }), { status: HttpStatus.OK, @@ -145,7 +98,7 @@ describe('openCdsClientService', () => { const [url, init] = getLastFetchCall() expect(url).toBe('https://opencds.example.com/root/api/validate/id') - expect(init.dispatcher?.dispatch).toBe(tlsDispatcher) + expect(init.dispatcher).toBeUndefined() expect(init.method).toBe('POST') expect(init.signal).toBeUndefined() expect(init.body).toBeUndefined() @@ -167,7 +120,7 @@ describe('openCdsClientService', () => { requestId: '123', enabled: true, })) - expect(init.dispatcher?.dispatch).toBe(tlsDispatcher) + expect(init.dispatcher).toBeUndefined() expect(init.method).toBe('POST') expect(init.signal).toBeUndefined() expect(new Headers(init.headers).get('X-API-Key')).toBe('test-token') @@ -194,4 +147,29 @@ describe('openCdsClientService', () => { statusText: 'Bad Gateway', }) }) + + it('uses a local Agent with rejectUnauthorized:false when TLS verification is disabled', async () => { + config = { + ...config, + openCdsApiTlsRejectUnauthorized: false, + } + module = await Test.createTestingModule({ + providers: [ + OpenCdsClientService, + { provide: ConfigurationService, useValue: config }, + ], + }).compile() + service = module.get(OpenCdsClientService) + + mockFetchResponse(new Response(JSON.stringify({ ok: true }), { + status: HttpStatus.OK, + headers: { 'content-type': 'application/json' }, + })) + + await service.get<{ ok: boolean }>('/requests') + + expect(Agent).toHaveBeenCalledWith({ connect: { rejectUnauthorized: false } }) + const [, init] = getLastFetchCall() + expect(init.dispatcher).toBeDefined() + }) }) diff --git a/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.ts b/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.ts index b018e8d450..38dc80e8f7 100644 --- a/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.ts +++ b/apps/server-nestjs/src/modules/service-chain/open-cds-client.service.ts @@ -1,7 +1,7 @@ import type { HttpStatus } from '@nestjs/common' import type { Dispatcher, HeadersInit } from 'undici' import { Inject, Injectable, Logger } from '@nestjs/common' -import { Agent, fetch, Headers, ProxyAgent } from 'undici' +import { Agent, fetch, Headers } from 'undici' import { ConfigurationService } from '../infrastructure/configuration/configuration.service' import { throwIfNotOk } from './service-chain.utils' @@ -110,20 +110,13 @@ export class OpenCdsClientService { return mergedHeaders } - private buildDispatcher(): Dispatcher { - if (process.env.HTTP_PROXY) { - return new ProxyAgent({ - requestTls: { - rejectUnauthorized: this.config.openCdsApiTlsRejectUnauthorized, - }, - uri: process.env.HTTP_PROXY, - }) + private buildDispatcher(): Dispatcher | undefined { + // Only the TLS-verify-disabled case needs a local dispatcher; proxy bypass on + // that rare path is accepted. Security default (apiTlsRejectUnauthorized=true) keeps cert verification ON. + if (!this.config.openCdsApiTlsRejectUnauthorized) { + return new Agent({ connect: { rejectUnauthorized: false } }) } - return new Agent({ - connect: { - rejectUnauthorized: this.config.openCdsApiTlsRejectUnauthorized, - }, - }) + return undefined } } diff --git a/apps/server/src/runtime.ts b/apps/server/src/runtime.ts index d5b8822533..aa15adc664 100644 --- a/apps/server/src/runtime.ts +++ b/apps/server/src/runtime.ts @@ -1,18 +1,17 @@ -import { Agent, cacheStores, interceptors, ProxyAgent, setGlobalDispatcher } from 'undici' +import { cacheStores, EnvHttpProxyAgent, interceptors, setGlobalDispatcher } from 'undici' -const base = process.env.HTTP_PROXY ? new ProxyAgent(process.env.HTTP_PROXY) : new Agent() +// Undici's EnvHttpProxyAgent reads HTTP_PROXY / HTTPS_PROXY / NO_PROXY from the +// environment and routes per-URL. NO_PROXY supports `host:port` entries and `*` +// (never proxy). Replaces the old `HTTP_PROXY ? ProxyAgent : Agent` branch. +const base = new EnvHttpProxyAgent() -// Undici’s cache interceptor follows RFC 7234: +// Undici's cache interceptor follows RFC 7234: // 1. Only GET/HEAD are cached (configurable via `methods`). -// 2. Cache-Control directives are obeyed: -// - `no-store` → never cache -// - `max-age`, `s-maxage`, `expires` → freshness lifetime -// - `private` → skip if shared store -// - `no-cache` → revalidate with ETag/Last-Modified -// 3. Heuristic caching (status 200 without explicit freshness) -// uses 10 % of time since Last-Modified if present. -// 4. Stale responses are served while revalidating in background -// when `stale-while-revalidate` is present. +// 2. Cache-Control directives are obeyed (no-store, max-age, private, no-cache). +// 3. Heuristic caching (status 200 without explicit freshness) uses 10% of +// time since Last-Modified if present. +// 4. Stale responses are served while revalidating in background when +// `stale-while-revalidate` is present. const client = base.compose( interceptors.cache({ store: new cacheStores.SqliteCacheStore(),