Do not overwrite https.Agent certificates (microsoft/vscode#234175)

CHANGELOG.md

@@ -1,6 +1,9 @@
 # Change Log
 Notable changes will be documented here.
 
+## [0.25.0]
+- Do not overwrite https.Agent certificates ([microsoft/vscode#234175](https://github.com/microsoft/vscode/issues/234175))
+
 ## [0.24.0]
 - Skip keepAlive flag ([microsoft/vscode#228872](https://github.com/microsoft/vscode/issues/228872))
 - Refactor for reuse with fetch ([microsoft/vscode#228697](https://github.com/microsoft/vscode/issues/228697))

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@vscode/proxy-agent",
-  "version": "0.24.0",
+  "version": "0.25.0",
   "description": "NodeJS http(s) agent implementation for VS Code",
   "main": "out/index.js",
   "types": "out/index.d.ts",

src/index.ts

@@ -363,7 +363,7 @@ export function createHttpPatch(params: ProxyAgentParams, originals: typeof http
 			const optionsPatched = originalAgent instanceof PacProxyAgent;
 			const config = params.getProxySupport();
 			const useProxySettings = !optionsPatched && (config === 'override' || config === 'fallback' || (config === 'on' && originalAgent === undefined));
-			const addCertificatesV1 = !optionsPatched && params.addCertificatesV1() && isHttps && !(options as https.RequestOptions).ca;
+			const addCertificatesV1 = !optionsPatched && params.addCertificatesV1() && isHttps && !(options as https.RequestOptions).ca && !(originalAgent instanceof originals.Agent && (originalAgent as https.Agent).options?.ca);
 
 			if (useProxySettings || addCertificatesV1) {
 				if (url) {
@@ -402,6 +402,7 @@ export function createHttpPatch(params: ProxyAgentParams, originals: typeof http
 
 export interface SecureContextOptionsPatch {
 	_vscodeAdditionalCaCerts?: (string | Buffer)[];
+	_vscodeTestReplaceCaCerts?: boolean;
 }
 
 export function createNetPatch(params: ProxyAgentParams, originals: typeof net) {
@@ -553,7 +554,11 @@ function addCertificatesV1(params: ProxyAgentParams, addCertificatesV1: boolean,
 	if (addCertificatesV1) {
 		getOrLoadAdditionalCertificates(params)
 			.then(caCertificates => {
-				(opts as SecureContextOptionsPatch)._vscodeAdditionalCaCerts = caCertificates;
+				if ((opts as SecureContextOptionsPatch)._vscodeTestReplaceCaCerts) {
+					(opts as https.RequestOptions).ca = caCertificates;
+				} else {
+					(opts as SecureContextOptionsPatch)._vscodeAdditionalCaCerts = caCertificates;
+				}
 				callback();
 			})
 			.catch(err => {

tests/test-client/src/direct.test.ts

@@ -2,7 +2,7 @@ import * as http from 'http';
 import * as https from 'https';
 import * as vpa from '../../..';
 import { createPacProxyAgent } from '../../../src/agent';
-import { testRequest, ca, directProxyAgentParams } from './utils';
+import { testRequest, ca, directProxyAgentParams, unusedCa, directProxyAgentParamsV1 } from './utils';
 import * as assert from 'assert';
 
 describe('Direct client', function () {
@@ -176,4 +176,53 @@ describe('Direct client', function () {
 			}
 		});
 	});
+	
+	it('should use system certificates', async function () {
+		const { resolveProxyWithRequest: resolveProxy } = vpa.createProxyResolver(directProxyAgentParamsV1);
+		const patchedHttps: typeof https = {
+			...https,
+			...vpa.createHttpPatch(directProxyAgentParamsV1, https, resolveProxy),
+		} as any;
+		await testRequest(patchedHttps, {
+			hostname: 'test-https-server',
+			path: '/test-path',
+			_vscodeTestReplaceCaCerts: true,
+		});
+	});
+	it('should use ca request option', async function () {
+		const { resolveProxyWithRequest: resolveProxy } = vpa.createProxyResolver(directProxyAgentParamsV1);
+		const patchedHttps: typeof https = {
+			...https,
+			...vpa.createHttpPatch(directProxyAgentParamsV1, https, resolveProxy),
+		} as any;
+		try {
+			await testRequest(patchedHttps, {
+				hostname: 'test-https-server',
+				path: '/test-path',
+				_vscodeTestReplaceCaCerts: true,
+				ca: unusedCa,
+			});
+			assert.fail('Expected to fail with self-signed certificate');
+		} catch (err: any) {
+			assert.strictEqual(err?.message, 'self-signed certificate');
+		}
+	});
+	it('should use ca agent option', async function () {
+		const { resolveProxyWithRequest: resolveProxy } = vpa.createProxyResolver(directProxyAgentParamsV1);
+		const patchedHttps: typeof https = {
+			...https,
+			...vpa.createHttpPatch(directProxyAgentParamsV1, https, resolveProxy),
+		} as any;
+		try {
+			await testRequest(patchedHttps, {
+				hostname: 'test-https-server',
+				path: '/test-path',
+				_vscodeTestReplaceCaCerts: true,
+				agent: new https.Agent({ ca: unusedCa }),
+			});
+			assert.fail('Expected to fail with self-signed certificate');
+		} catch (err: any) {
+			assert.strictEqual(err?.message, 'self-signed certificate');
+		}
+	});
 });

tests/test-client/src/utils.ts

@@ -12,12 +12,14 @@ export const ca = [
 	fs.readFileSync(path.join(__dirname, '../../test-https-server/ssl_teapot_cert.pem')).toString(),
 ];
 
+export const unusedCa = fs.readFileSync(path.join(__dirname, '../../test-https-server/ssl_unused_cert.pem')).toString();
+
 export const directProxyAgentParams: vpa.ProxyAgentParams = {
 	resolveProxy: async () => 'DIRECT',
-    getProxyURL: () => undefined,
-    getProxySupport: () => 'override',
-    addCertificatesV1: () => false,
-    addCertificatesV2: () => true,
+	getProxyURL: () => undefined,
+	getProxySupport: () => 'override',
+	addCertificatesV1: () => false,
+	addCertificatesV2: () => true,
 	log: console,
 	getLogLevel: () => vpa.LogLevel.Trace,
 	proxyResolveTelemetry: () => undefined,
@@ -29,7 +31,13 @@ export const directProxyAgentParams: vpa.ProxyAgentParams = {
 	env: {},
 };
 
-export async function testRequest<C extends typeof https | typeof http>(client: C, options: C extends typeof https ? https.RequestOptions : http.RequestOptions, testOptions: { assertResult?: (result: any, req: http.ClientRequest, res: http.IncomingMessage) => void; } = {}) {
+export const directProxyAgentParamsV1: vpa.ProxyAgentParams = {
+	...directProxyAgentParams,
+	addCertificatesV1: () => true,
+	addCertificatesV2: () => false,
+};
+
+export async function testRequest<C extends typeof https | typeof http>(client: C, options: C extends typeof https ? (https.RequestOptions & vpa.SecureContextOptionsPatch) : http.RequestOptions, testOptions: { assertResult?: (result: any, req: http.ClientRequest, res: http.IncomingMessage) => void; } = {}) {
 	return new Promise<void>((resolve, reject) => {
 		const req = client.request(options, res => {
 			if (!res.statusCode || res.statusCode < 200 || res.statusCode > 299) {

tests/test-https-server/Dockerfile

@@ -8,6 +8,7 @@ WORKDIR /etc/nginx
 
 CMD openssl req -nodes -x509 -newkey rsa:2048 -keyout ssl_key.pem -out ssl_cert.pem -days 365 -subj "/C=US/ST=Oregon/L=Portland/O=IT/CN=test-https-server" \
     && openssl req -nodes -x509 -newkey rsa:2048 -keyout ssl_teapot_key.pem -out ssl_teapot_cert.pem -days 365 -subj "/C=US/ST=Oregon/L=Portland/O=IT/CN=test-teapot-server" \
+    && openssl req -nodes -x509 -newkey rsa:2048 -keyout ssl_unused_key.pem -out ssl_unused_cert.pem -days 365 -subj "/C=US/ST=Oregon/L=Portland/O=IT/CN=test-https-server" \
     && nginx -g 'daemon off;'
 
 HEALTHCHECK --interval=1s --timeout=3s --retries=10 \