Remove dependency on node-forge and run test as electron

Author: EhabY

package.json

@@ -27,6 +27,7 @@
 		"pretest": "tsc -p . --outDir out && tsc -p test --outDir out && yarn run build && yarn run lint",
 		"test": "vitest",
 		"test:ci": "CI=true yarn test",
+		"test:electron": "ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs test/unit/error.test.ts",
 		"test:integration": "vscode-test",
 		"vscode:prepublish": "yarn package",
 		"watch": "webpack --watch"
@@ -343,12 +344,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 +362,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 +375,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,38 @@ 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:
+						// We use "@peculiar/x509" 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)) {
+						const cert = new X509Certificate(x509.toString());
+						const isSelfIssued = await cert.isSelfSigned();
+						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,11 +12,8 @@ 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).
+	// Some tests behave differently in Electron (BoringSSL) vs Node.js (OpenSSL).
+	// Run `yarn test:electron` to test Electron-specific behavior.
 	const isElectron =
 		(process.versions.electron || process.env.ELECTRON_RUN_AS_NODE) &&
 		!process.env.VSCODE_PID; // Running from the test explorer in VS Code