Review comments

Author: EhabY

src/error.ts

@@ -80,30 +80,20 @@ export class CertificateError extends Error {
 				const url = new URL(address);
 				const socket = tls.connect(
 					{
-						port: parseInt(url.port, 10) || 443,
+						port: Number.parseInt(url.port, 10) || 443,
 						host: url.hostname,
 						rejectUnauthorized: false,
 					},
-					async () => {
+					() => {
 						const x509 = socket.getPeerX509Certificate();
 						socket.destroy();
 						if (!x509) {
 							throw new Error("no peer certificate");
 						}
 
-						/**
-						 * 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.
-						 */
+						// We use "@peculiar/x509" because Node's x509 returns an undefined `keyUsage`.
 						const cert = new X509Certificate(x509.toString());
-						let isSelfIssued: boolean;
-						try {
-							isSelfIssued = await cert.isSelfSigned();
-						} catch (err) {
-							return reject(err);
-						}
+						const isSelfIssued = cert.subject === cert.issuer;
 						if (!isSelfIssued) {
 							return resolve(X509_ERR.PARTIAL_CHAIN);
 						}

src/remote/remote.ts

@@ -344,6 +344,7 @@ export class Remote {
 								} catch (error) {
 									subscription.dispose();
 									reject(error);
+									return;
 								} finally {
 									inProgress = false;
 								}

test/unit/error.test.ts

@@ -1,4 +1,9 @@
+import {
+	KeyUsagesExtension,
+	X509Certificate as X509CertificatePeculiar,
+} from "@peculiar/x509";
 import axios from "axios";
+import { X509Certificate as X509CertificateNode } from "node:crypto";
 import * as fs from "node:fs/promises";
 import https from "node:https";
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
@@ -15,9 +20,7 @@ describe("Certificate errors", () => {
 	// 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();
+		expect(process.versions.electron).toBeTruthy();
 	});
 
 	beforeAll(() => {
@@ -113,8 +116,7 @@ describe("Certificate errors", () => {
 	});
 
 	// In Electron a self-issued certificate without the signing capability fails
-	// (again with the same "unable to verify" error) but in Node self-issued
-	// certificates are not required to have the signing capability.
+	// (again with the same "unable to verify" error)
 	it("detects self-signed certificates without signing capability", async () => {
 		const address = await startServer("no-signing");
 		const request = axios.get(address, {
@@ -146,6 +148,24 @@ describe("Certificate errors", () => {
 		await expect(request).resolves.toHaveProperty("data", "foobar");
 	});
 
+	// Node's X509Certificate.keyUsage is unreliable, so use a third-party parser
+	it("parses no-signing cert keyUsage with third-party library", async () => {
+		const certPem = await fs.readFile(
+			getFixturePath("tls", "no-signing.crt"),
+			"utf-8",
+		);
+
+		// Node's implementation seems to always return `undefined`
+		const nodeCert = new X509CertificateNode(certPem);
+		expect(nodeCert.keyUsage).toBeUndefined();
+
+		// Here we can correctly get the KeyUsages
+		const peculiarCert = new X509CertificatePeculiar(certPem);
+		const extension = peculiarCert.getExtension(KeyUsagesExtension);
+		expect(extension).toBeDefined();
+		expect(extension?.usages).toBeTruthy();
+	});
+
 	// Both environments give the same error code when a self-issued certificate is
 	// untrusted.
 	it("detects self-signed certificates", async () => {