Remove node forge dependency

.vscode/settings.json

@@ -10,5 +10,9 @@
 	},
 	"[jsonc]": {
 		"editor.defaultFormatter": "esbenp.prettier-vscode"
-	}
+	},
+	"vitest.nodeEnv": {
+		"ELECTRON_RUN_AS_NODE": "1"
+	},
+	"vitest.nodeExecutable": "node_modules/.bin/electron"
 }

package.json

@@ -25,7 +25,7 @@
 		"package": "webpack --mode production --devtool hidden-source-map",
 		"package:prerelease": "npx vsce package --pre-release",
 		"pretest": "tsc -p . --outDir out && tsc -p test --outDir out && yarn run build && yarn run lint",
-		"test": "vitest",
+		"test": "ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs",
 		"test:ci": "CI=true yarn test",
 		"test:integration": "vscode-test",
 		"vscode:prepublish": "yarn package",
@@ -343,12 +343,12 @@
 		"word-wrap": "1.2.5"
 	},
 	"dependencies": {
+		"@peculiar/x509": "^1.14.0",
 		"axios": "1.12.2",
 		"date-fns": "^3.6.0",
 		"eventsource": "^3.0.6",
 		"find-process": "https://github.com/coder/find-process#fix/sequoia-compat",
 		"jsonc-parser": "^3.3.1",
-		"node-forge": "^1.3.1",
 		"openpgp": "^6.2.2",
 		"pretty-bytes": "^7.1.0",
 		"proxy-agent": "^6.5.0",
@@ -361,7 +361,6 @@
 		"@types/eventsource": "^3.0.0",
 		"@types/glob": "^7.1.3",
 		"@types/node": "^22.14.1",
-		"@types/node-forge": "^1.3.14",
 		"@types/semver": "^7.7.1",
 		"@types/ua-parser-js": "0.7.36",
 		"@types/vscode": "^1.73.0",
@@ -375,6 +374,7 @@
 		"bufferutil": "^4.0.9",
 		"coder": "https://github.com/coder/coder#main",
 		"dayjs": "^1.11.13",
+		"electron": "^39.1.2",
 		"eslint": "^8.57.1",
 		"eslint-config-prettier": "^10.1.8",
 		"eslint-import-resolver-typescript": "^4.4.4",

src/error.ts

@@ -1,7 +1,11 @@
+import {
+	X509Certificate,
+	KeyUsagesExtension,
+	KeyUsageFlags,
+} from "@peculiar/x509";
 import { isAxiosError } from "axios";
 import { isApiError, isApiErrorResponse } from "coder/site/src/api/errors";
-import * as forge from "node-forge";
-import * as tls from "tls";
+import * as tls from "node:tls";
 import * as vscode from "vscode";
 
 import { type Logger } from "./logging/logger";
@@ -23,10 +27,6 @@ export enum X509_ERR {
 	UNTRUSTED_CHAIN = "Your Coder deployment's certificate chain does not appear to be trusted by this system. The root of the certificate chain must be added to this system's trust store. ",
 }
 
-interface KeyUsage {
-	keyCertSign: boolean;
-}
-
 export class CertificateError extends Error {
 	public static ActionAllowInsecure = "Allow Insecure";
 	public static ActionOK = "OK";
@@ -84,36 +84,44 @@ export class CertificateError extends Error {
 						host: url.hostname,
 						rejectUnauthorized: false,
 					},
-					() => {
+					async () => {
 						const x509 = socket.getPeerX509Certificate();
 						socket.destroy();
 						if (!x509) {
 							throw new Error("no peer certificate");
 						}
 
-						// We use node-forge for two reasons:
-						// 1. Node/Electron only provide extended key usage.
-						// 2. Electron's checkIssued() will fail because it suffers from same
-						//    the key usage bug that we are trying to work around here in the
-						//    first place.
-						const cert = forge.pki.certificateFromPem(x509.toString());
-						if (!cert.issued(cert)) {
+						/**
+						 * We use "@peculiar/x509" for two reasons:
+						 * 1. Node's x509 returns an undefined `keyUsage` in Electron environments.
+						 * 2. Electron's checkIssued() will fail because it suffers from same
+						 *    the key usage bug that we are trying to work around here in the first place.
+						 */
+						const cert = new X509Certificate(x509.toString());
+						let isSelfIssued: boolean;
+						try {
+							isSelfIssued = await cert.isSelfSigned();
+						} catch (err) {
+							return reject(err);
+						}
+						if (!isSelfIssued) {
 							return resolve(X509_ERR.PARTIAL_CHAIN);
 						}
 
 						// The key usage needs to exist but not have cert signing to fail.
-						const keyUsage = cert.getExtension({ name: "keyUsage" }) as
-							| KeyUsage
-							| undefined;
-						if (keyUsage && !keyUsage.keyCertSign) {
-							return resolve(X509_ERR.NON_SIGNING);
-						} else {
-							// This branch is currently untested; it does not appear possible to
-							// get the error "unable to verify" with a self-signed certificate
-							// unless the key usage was the issue since it would have errored
-							// with "self-signed certificate" instead.
-							return resolve(X509_ERR.UNTRUSTED_LEAF);
+						const extension = cert.getExtension(KeyUsagesExtension);
+						if (extension) {
+							const hasKeyCertSign =
+								extension.usages & KeyUsageFlags.keyCertSign;
+							if (!hasKeyCertSign) {
+								return resolve(X509_ERR.NON_SIGNING);
+							}
 						}
+						// This branch is currently untested; it does not appear possible to
+						// get the error "unable to verify" with a self-signed certificate
+						// unless the key usage was the issue since it would have errored
+						// with "self-signed certificate" instead.
+						return resolve(X509_ERR.UNTRUSTED_LEAF);
 					},
 				);
 				socket.on("error", reject);

test/unit/error.test.ts

@@ -1,6 +1,6 @@
 import axios from "axios";
-import * as fs from "fs/promises";
-import https from "https";
+import * as fs from "node:fs/promises";
+import https from "node:https";
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
 
 import { CertificateError, X509_ERR, X509_ERR_CODE } from "@/error";
@@ -12,14 +12,13 @@ describe("Certificate errors", () => {
 	// Before each test we make a request to sanity check that we really get the
 	// error we are expecting, then we run it through CertificateError.
 
-	// TODO: These sanity checks need to be ran in an Electron environment to
-	// reflect real usage in VS Code.  We should either revert back to the standard
-	// extension testing framework which I believe runs in a headless VS Code
-	// instead of using vitest or at least run the tests through Electron running as
-	// Node (for now I do this manually by shimming Node).
-	const isElectron =
-		(process.versions.electron || process.env.ELECTRON_RUN_AS_NODE) &&
-		!process.env.VSCODE_PID; // Running from the test explorer in VS Code
+	// These tests run in Electron (BoringSSL) for accurate certificate validation testing.
+
+	it("should run in Electron environment", () => {
+		const isElectron =
+			process.versions.electron || process.env.ELECTRON_RUN_AS_NODE;
+		expect(isElectron).toBeTruthy();
+	});
 
 	beforeAll(() => {
 		vi.mock("vscode", () => {
@@ -124,26 +123,16 @@ describe("Certificate errors", () => {
 				servername: "localhost",
 			}),
 		});
-		if (isElectron) {
-			await expect(request).rejects.toHaveProperty(
-				"code",
-				X509_ERR_CODE.UNABLE_TO_VERIFY_LEAF_SIGNATURE,
-			);
-			try {
-				await request;
-			} catch (error) {
-				const wrapped = await CertificateError.maybeWrap(
-					error,
-					address,
-					logger,
-				);
-				expect(wrapped instanceof CertificateError).toBeTruthy();
-				expect((wrapped as CertificateError).x509Err).toBe(
-					X509_ERR.NON_SIGNING,
-				);
-			}
-		} else {
-			await expect(request).resolves.toHaveProperty("data", "foobar");
+		await expect(request).rejects.toHaveProperty(
+			"code",
+			X509_ERR_CODE.UNABLE_TO_VERIFY_LEAF_SIGNATURE,
+		);
+		try {
+			await request;
+		} catch (error) {
+			const wrapped = await CertificateError.maybeWrap(error, address, logger);
+			expect(wrapped instanceof CertificateError).toBeTruthy();
+			expect((wrapped as CertificateError).x509Err).toBe(X509_ERR.NON_SIGNING);
 		}
 	});