diff --git a/CHANGELOG.md b/CHANGELOG.md index e9ab6790..4cb53344 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,15 @@ ## Unreleased +### Added + +- Automatic TLS client certificate refresh via new `coder.tlsCertRefreshCommand` setting. Detects + certificate errors (expired, revoked, etc.) and automatically refreshes and retries. + ### Fixed - Fixed `SetEnv` SSH config parsing and accumulation with user-defined values. +- Improved WebSocket error handling for more consistent behavior across connection failures. ## [v1.11.6](https://github.com/coder/vscode-coder/releases/tag/v1.11.6) 2025-12-15 diff --git a/package.json b/package.json index 79445065..ec22ddea 100644 --- a/package.json +++ b/package.json @@ -92,6 +92,11 @@ "type": "string", "default": "" }, + "coder.tlsCertRefreshCommand": { + "markdownDescription": "Command to run when TLS client certificate errors occur (e.g., expired, revoked, or rejected certificates). If configured, the extension will automatically execute this command and retry failed requests. `http.proxySupport` must be set to `on` or `off`, otherwise VS Code will override the proxy agent set by the plugin.", + "type": "string", + "default": "" + }, "coder.proxyLogDirectory": { "markdownDescription": "If set, the Coder CLI will output extra SSH information into this directory, which can be helpful for debugging connectivity issues.", "type": "string", diff --git a/src/api/certificateRefresh.ts b/src/api/certificateRefresh.ts new file mode 100644 index 00000000..580691e1 --- /dev/null +++ b/src/api/certificateRefresh.ts @@ -0,0 +1,30 @@ +import * as vscode from "vscode"; + +import { execCommand } from "../command/exec"; +import { type Logger } from "../logging/logger"; + +/** + * Returns the configured certificate refresh command, or undefined if not set. + */ +export function getRefreshCommand(): string | undefined { + return ( + vscode.workspace + .getConfiguration() + .get("coder.tlsCertRefreshCommand") + ?.trim() || undefined + ); +} + +/** + * Executes the certificate refresh command. + * Returns true if successful, false otherwise. + */ +export async function refreshCertificates( + command: string, + logger: Logger, +): Promise { + const result = await execCommand(command, logger, { + title: "Certificate refresh", + }); + return result.success; +} diff --git a/src/api/coderApi.ts b/src/api/coderApi.ts index 4c457ab2..28c3b2bb 100644 --- a/src/api/coderApi.ts +++ b/src/api/coderApi.ts @@ -17,8 +17,9 @@ import * as vscode from "vscode"; import { type ClientOptions } from "ws"; import { watchConfigurationChanges } from "../configWatcher"; -import { CertificateError } from "../error/certificateError"; +import { ClientCertificateError } from "../error/clientCertificateError"; import { toError } from "../error/errorUtils"; +import { ServerCertificateError } from "../error/serverCertificateError"; import { getHeaderCommand, getHeaders } from "../headers"; import { EventStreamLogger } from "../logging/eventStreamLogger"; import { @@ -49,6 +50,7 @@ import { } from "../websocket/reconnectingWebSocket"; import { SseConnection } from "../websocket/sseConnection"; +import { getRefreshCommand, refreshCertificates } from "./certificateRefresh"; import { createHttpAgent } from "./utils"; const coderSessionTokenHeader = "Coder-Session-Token"; @@ -309,7 +311,9 @@ export class CoderApi extends Api implements vscode.Disposable { }); this.attachStreamLogger(ws); - return ws; + + // Wait for connection to open before returning + return await this.waitForOpen(ws); } private attachStreamLogger( @@ -349,9 +353,8 @@ export class CoderApi extends Api implements vscode.Disposable { ): Promise> { const { fallbackApiRoute, ...socketConfigs } = configs; try { - const ws = - await this.createOneWayWebSocket(socketConfigs); - return await this.waitForOpen(ws); + // createOneWayWebSocket already waits for open + return await this.createOneWayWebSocket(socketConfigs); } catch (error) { if (this.is404Error(error)) { this.output.warn( @@ -396,10 +399,11 @@ export class CoderApi extends Api implements vscode.Disposable { /** * Wait for a connection to open. Rejects on error. + * Preserves the specific connection type (e.g., OneWayWebSocket, SseConnection). */ - private waitForOpen( - connection: UnidirectionalStream, - ): Promise> { + private waitForOpen>( + connection: T, + ): Promise { return new Promise((resolve, reject) => { const cleanup = () => { connection.removeEventListener("open", handleOpen); @@ -414,7 +418,10 @@ export class CoderApi extends Api implements vscode.Disposable { const handleError = (event: ErrorEvent) => { cleanup(); connection.close(); - const error = toError(event.error, "WebSocket connection error"); + const error = toError( + event.error, + event.message || "WebSocket connection error", + ); reject(error); }; @@ -440,7 +447,15 @@ export class CoderApi extends Api implements vscode.Disposable { const reconnectingSocket = await ReconnectingWebSocket.create( socketFactory, this.output, - undefined, + { + onCertificateRefreshNeeded: async () => { + const refreshCommand = getRefreshCommand(); + if (!refreshCommand) { + return false; + } + return refreshCertificates(refreshCommand, this.output); + }, + }, () => this.reconnectingSockets.delete(reconnectingSocket), ); @@ -479,16 +494,53 @@ function setupInterceptors(client: CoderApi, output: Logger): void { return config; }); - // Wrap certificate errors. + // Wrap certificate errors and handle client certificate errors with refresh. client.getAxiosInstance().interceptors.response.use( (r) => r, - async (err) => { + async (err: unknown) => { + const refreshCommand = getRefreshCommand(); + const certError = ClientCertificateError.fromError(err); + if (certError) { + if (certError.isRefreshable && refreshCommand) { + const config = ( + err as { + config?: RequestConfigWithMeta & { + _certRetried?: boolean; + }; + } + ).config; + + if (config && !config._certRetried) { + config._certRetried = true; + + output.info( + `Client certificate error (alert ${certError.alertCode}), attempting refresh...`, + ); + const success = await refreshCertificates(refreshCommand, output); + if (success) { + // Create new agent with refreshed certificates. + const agent = await createHttpAgent( + vscode.workspace.getConfiguration(), + ); + config.httpsAgent = agent; + config.httpAgent = agent; + + // Retry the request. + output.info("Retrying request with refreshed certificates..."); + return client.getAxiosInstance().request(config); + } + } + } + + throw certError; + } + + // Handle other certificate errors. const baseUrl = client.getAxiosInstance().defaults.baseURL; if (baseUrl) { - throw await CertificateError.maybeWrap(err, baseUrl, output); - } else { - throw err; + throw await ServerCertificateError.maybeWrap(err, baseUrl, output); } + throw err; }, ); } diff --git a/src/command/exec.ts b/src/command/exec.ts new file mode 100644 index 00000000..6e43d040 --- /dev/null +++ b/src/command/exec.ts @@ -0,0 +1,74 @@ +import * as cp from "node:child_process"; +import * as util from "node:util"; + +import { type Logger } from "../logging/logger"; + +interface ExecException { + code?: number; + stderr?: string; + stdout?: string; +} + +function isExecException(err: unknown): err is ExecException { + return (err as ExecException).code !== undefined; +} + +export interface ExecCommandOptions { + env?: NodeJS.ProcessEnv; + /** Title for logging (e.g., "Header command", "Certificate refresh"). */ + title?: string; +} + +export type ExecCommandResult = + | { success: true; stdout: string; stderr: string } + | { success: false; stdout?: string; stderr?: string; exitCode?: number }; + +/** + * Execute a shell command and return result with success/failure. + * Handles errors gracefully and logs appropriately. + */ +export async function execCommand( + command: string, + logger: Logger, + options?: ExecCommandOptions, +): Promise { + const title = options?.title ?? "Command"; + logger.debug(`Executing ${title}: ${command}`); + + try { + const result = await util.promisify(cp.exec)(command, { + env: options?.env, + }); + logger.debug(`${title} completed successfully`); + if (result.stdout) { + logger.debug(`${title} stdout:`, result.stdout); + } + if (result.stderr) { + logger.debug(`${title} stderr:`, result.stderr); + } + return { + success: true, + stdout: result.stdout, + stderr: result.stderr, + }; + } catch (error) { + if (isExecException(error)) { + logger.warn(`${title} failed with exit code ${error.code}`); + if (error.stdout) { + logger.warn(`${title} stdout:`, error.stdout); + } + if (error.stderr) { + logger.warn(`${title} stderr:`, error.stderr); + } + return { + success: false, + stdout: error.stdout, + stderr: error.stderr, + exitCode: error.code, + }; + } + + logger.warn(`${title} failed:`, error); + return { success: false }; + } +} diff --git a/src/error/certificateError.ts b/src/error/certificateError.ts index c674c30e..57f38e14 100644 --- a/src/error/certificateError.ts +++ b/src/error/certificateError.ts @@ -1,164 +1,32 @@ -import { - X509Certificate, - KeyUsagesExtension, - KeyUsageFlags, -} from "@peculiar/x509"; -import { isAxiosError } from "axios"; -import * as tls from "node:tls"; import * as vscode from "vscode"; -import { type Logger } from "../logging/logger"; +/** + * Base class for certificate-related errors that can display notifications to users. + * Use `instanceof CertificateError` to check if an error is a certificate error. + */ +export abstract class CertificateError extends Error { + /** Human-friendly detail message for display */ + public abstract readonly detail: string; -import { toError } from "./errorUtils"; - -// X509_ERR_CODE represents error codes as returned from BoringSSL/OpenSSL. -export enum X509_ERR_CODE { - UNABLE_TO_VERIFY_LEAF_SIGNATURE = "UNABLE_TO_VERIFY_LEAF_SIGNATURE", - DEPTH_ZERO_SELF_SIGNED_CERT = "DEPTH_ZERO_SELF_SIGNED_CERT", - SELF_SIGNED_CERT_IN_CHAIN = "SELF_SIGNED_CERT_IN_CHAIN", -} - -// X509_ERR contains human-friendly versions of TLS errors. -export enum X509_ERR { - PARTIAL_CHAIN = "Your Coder deployment's certificate cannot be verified because a certificate is missing from its chain. To fix this your deployment's administrator must bundle the missing certificates.", - // NON_SIGNING can be removed if BoringSSL is patched and the patch makes it - // into the version of Electron used by VS Code. - NON_SIGNING = "Your Coder deployment's certificate is not marked as being capable of signing. VS Code uses a version of Electron that does not support certificates like this even if they are self-issued. The certificate must be regenerated with the certificate signing capability.", - UNTRUSTED_LEAF = "Your Coder deployment's certificate does not appear to be trusted by this system. The certificate must be added to this system's trust store.", - 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. ", -} - -export class CertificateError extends Error { - public static readonly ActionAllowInsecure = "Allow Insecure"; - public static readonly ActionOK = "OK"; - public static readonly InsecureMessage = - 'The Coder extension will no longer verify TLS on HTTPS requests. You can change this at any time with the "coder.insecure" property in your VS Code settings.'; - - private constructor( - message: string, - public readonly x509Err?: X509_ERR, - ) { - super("Secure connection to your Coder deployment failed: " + message); - } - - // maybeWrap returns a CertificateError if the code is a certificate error - // otherwise it returns the original error. - static async maybeWrap( - err: T, - address: string, - logger: Logger, - ): Promise { - if (isAxiosError(err)) { - switch (err.code) { - case X509_ERR_CODE.UNABLE_TO_VERIFY_LEAF_SIGNATURE: - // "Unable to verify" can mean different things so we will attempt to - // parse the certificate and determine which it is. - try { - const cause = - await CertificateError.determineVerifyErrorCause(address); - return new CertificateError(err.message, cause); - } catch (error) { - logger.warn(`Failed to parse certificate from ${address}`, error); - break; - } - case X509_ERR_CODE.DEPTH_ZERO_SELF_SIGNED_CERT: - return new CertificateError(err.message, X509_ERR.UNTRUSTED_LEAF); - case X509_ERR_CODE.SELF_SIGNED_CERT_IN_CHAIN: - return new CertificateError(err.message, X509_ERR.UNTRUSTED_CHAIN); - case undefined: - break; - } - } - return err; - } - - // determineVerifyErrorCause fetches the certificate(s) from the specified - // address, parses the leaf, and returns the reason the certificate is giving - // an "unable to verify" error or throws if unable to figure it out. - private static async determineVerifyErrorCause( - address: string, - ): Promise { - return new Promise((resolve, reject) => { - try { - const url = new URL(address); - const socket = tls.connect( - { - port: Number.parseInt(url.port, 10) || 443, - host: url.hostname, - rejectUnauthorized: false, - }, - () => { - const x509 = socket.getPeerX509Certificate(); - socket.destroy(); - if (!x509) { - throw new Error("no peer certificate"); - } - - // We use "@peculiar/x509" because Node's x509 returns an undefined `keyUsage`. - const cert = new X509Certificate(x509.toString()); - const isSelfIssued = cert.subject === cert.issuer; - if (!isSelfIssued) { - return resolve(X509_ERR.PARTIAL_CHAIN); - } - - // The key usage needs to exist but not have cert signing to fail. - 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", (err) => reject(toError(err))); - } catch (err) { - reject(toError(err)); - } - }); - } - - // allowInsecure updates the value of the "coder.insecure" property. - private allowInsecure(): void { - vscode.workspace - .getConfiguration() - .update("coder.insecure", true, vscode.ConfigurationTarget.Global); - vscode.window.showInformationMessage(CertificateError.InsecureMessage); - } - - async showModal(title: string): Promise { - return this.showNotification(title, { - detail: this.x509Err || this.message, - modal: true, - useCustom: true, - }); - } + /** Show error notification. Pass { modal: true } for modal dialogs. */ + public abstract showNotification( + title?: string, + options?: { modal?: boolean }, + ): Promise; - async showNotification( + protected async showErrorMessage( title?: string, - options: vscode.MessageOptions = {}, - ): Promise { - const val = await vscode.window.showErrorMessage( - title || this.x509Err || this.message, - options, - // TODO: The insecure setting does not seem to work, even though it - // should, as proven by the tests. Even hardcoding rejectUnauthorized to - // false does not work; something seems to just be different when ran - // inside VS Code. Disabling the "Strict SSL" setting does not help - // either. For now avoid showing the button until this is sorted. - // CertificateError.ActionAllowInsecure, - CertificateError.ActionOK, + options?: { modal?: boolean }, + ...items: T[] + ): Promise { + const modal = options?.modal ?? false; + const message = + !modal && title ? `${title}: ${this.detail}` : title || this.detail; + + return await vscode.window.showErrorMessage( + message, + { modal, useCustom: modal, detail: this.detail }, + ...(items ?? []), ); - switch (val) { - case CertificateError.ActionOK: - case undefined: - return; - } } } diff --git a/src/error/clientCertificateError.ts b/src/error/clientCertificateError.ts new file mode 100644 index 00000000..32f9c804 --- /dev/null +++ b/src/error/clientCertificateError.ts @@ -0,0 +1,175 @@ +import * as vscode from "vscode"; + +import { CertificateError } from "./certificateError"; +import { toError } from "./errorUtils"; + +/** + * SSL/TLS alert codes related to client certificates (RFC 5246). + */ +export enum CLIENT_CERT_ALERT { + BAD_CERTIFICATE = 42, + UNSUPPORTED_CERTIFICATE = 43, + CERTIFICATE_REVOKED = 44, + CERTIFICATE_EXPIRED = 45, + CERTIFICATE_UNKNOWN = 46, + UNKNOWN_CA = 48, + ACCESS_DENIED = 49, +} + +/** + * User-friendly messages for each client certificate alert code. + */ +export const CLIENT_CERT_MESSAGES: Record = { + [CLIENT_CERT_ALERT.BAD_CERTIFICATE]: + "Your TLS client certificate appears to be corrupted or has invalid signatures.", + [CLIENT_CERT_ALERT.UNSUPPORTED_CERTIFICATE]: + "Your TLS client certificate type is not supported by the server.", + [CLIENT_CERT_ALERT.CERTIFICATE_REVOKED]: + "Your TLS client certificate has been revoked.", + [CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED]: + "Your TLS client certificate has expired.", + [CLIENT_CERT_ALERT.CERTIFICATE_UNKNOWN]: + "Your TLS client certificate was rejected for an unspecified reason.", + [CLIENT_CERT_ALERT.UNKNOWN_CA]: + "Your TLS client certificate was issued by an untrusted Certificate Authority.", + [CLIENT_CERT_ALERT.ACCESS_DENIED]: + "Access was denied using your TLS client certificate.", +}; + +/** + * Patterns to match SSL alert types in error messages (case-insensitive). + */ +const ALERT_PATTERNS: ReadonlyArray<[string, CLIENT_CERT_ALERT]> = + Object.entries(CLIENT_CERT_ALERT) + .filter( + (entry): entry is [string, CLIENT_CERT_ALERT] => + typeof entry[1] === "number", + ) + .map(([name, code]) => [name.toLowerCase(), code]); + +/** + * Alert codes that may be recoverable by refreshing the certificate. + */ +const REFRESHABLE_ALERTS: ReadonlySet = new Set([ + CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED, + CLIENT_CERT_ALERT.CERTIFICATE_REVOKED, + CLIENT_CERT_ALERT.BAD_CERTIFICATE, + CLIENT_CERT_ALERT.CERTIFICATE_UNKNOWN, +]); + +/** + * Error thrown when a TLS client certificate issue is detected. + * Provides user-friendly messages depending on the specific error type + * and whether a refresh command is configured. + */ +export class ClientCertificateError extends CertificateError { + private static readonly ActionConfigure = "Configure Refresh Command"; + + /** + * Extract error message string from various error types. + * Handles Error objects and plain strings. + */ + private static extractErrorMessage(err: unknown): string { + if (!err) { + return ""; + } + if (typeof err === "string") { + return err; + } + + const obj = err as { message?: string; code?: string }; + return [obj.code, obj.message].filter(Boolean).join(" "); + } + + /** + * Create a ClientCertificateError from any error type if it contains + * a recognized client certificate alert code. + * Returns undefined if the error is not a recognized client certificate error. + */ + public static fromError(err: unknown): ClientCertificateError | undefined { + const alertCode = this.detectAlertCode(err); + if (alertCode === undefined) { + return undefined; + } + + const baseMessage = CLIENT_CERT_MESSAGES[alertCode]; + const isRefreshable = REFRESHABLE_ALERTS.has(alertCode); + + const detail = isRefreshable + ? `${baseMessage} Try refreshing your credentials manually, or configure automatic certificate refresh in settings.` + : `${baseMessage} This issue cannot be resolved by refreshing the certificate. Please contact your administrator.`; + + return new ClientCertificateError( + toError(err), + alertCode, + detail, + isRefreshable, + ); + } + + /** + * Detect the SSL alert code from any error type. + * Returns undefined if the error is not a recognized client certificate error. + */ + private static detectAlertCode(err: unknown): CLIENT_CERT_ALERT | undefined { + const message = this.extractErrorMessage(err).toLowerCase(); + if (!message) { + return undefined; + } + + // Check for alert patterns (case-insensitive via lowercase) + for (const [pattern, alertCode] of ALERT_PATTERNS) { + if (message.includes(pattern)) { + return alertCode; + } + } + + // Fall back to parsing "ssl alert number XX" + const alertMatch = /ssl alert number (\d+)/.exec(message); + if (alertMatch) { + const alertNumber = Number.parseInt(alertMatch[1], 10); + if (alertNumber in CLIENT_CERT_ALERT) { + return alertNumber; + } + } + + return undefined; + } + + /** + * Check if an error is a refreshable client certificate error. + */ + public static isRefreshable(err: unknown): boolean { + const alertCode = this.detectAlertCode(err); + return alertCode !== undefined && REFRESHABLE_ALERTS.has(alertCode); + } + + private constructor( + public override readonly cause: Error, + public readonly alertCode: CLIENT_CERT_ALERT, + public readonly detail: string, + public readonly isRefreshable: boolean, + ) { + super(detail); + this.name = "ClientCertificateError"; + } + + async showNotification( + title?: string, + options?: { modal?: boolean }, + ): Promise { + // Only show the configure action for refreshable errors + const actions = this.isRefreshable + ? [ClientCertificateError.ActionConfigure] + : []; + + const val = await this.showErrorMessage(title, options, ...actions); + + if (val === ClientCertificateError.ActionConfigure) { + await vscode.commands.executeCommand( + "workbench.action.openSettings", + "coder.tlsCertRefreshCommand", + ); + } + } +} diff --git a/src/error/serverCertificateError.ts b/src/error/serverCertificateError.ts new file mode 100644 index 00000000..57723671 --- /dev/null +++ b/src/error/serverCertificateError.ts @@ -0,0 +1,170 @@ +import { + X509Certificate, + KeyUsagesExtension, + KeyUsageFlags, +} from "@peculiar/x509"; +import { isAxiosError } from "axios"; +import * as tls from "node:tls"; +import * as vscode from "vscode"; + +import { type Logger } from "../logging/logger"; + +import { CertificateError } from "./certificateError"; +import { toError } from "./errorUtils"; + +// X509_ERR_CODE represents error codes as returned from BoringSSL/OpenSSL. +export enum X509_ERR_CODE { + UNABLE_TO_VERIFY_LEAF_SIGNATURE = "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + DEPTH_ZERO_SELF_SIGNED_CERT = "DEPTH_ZERO_SELF_SIGNED_CERT", + SELF_SIGNED_CERT_IN_CHAIN = "SELF_SIGNED_CERT_IN_CHAIN", +} + +// X509_ERR contains human-friendly versions of TLS errors. +export enum X509_ERR { + PARTIAL_CHAIN = "Your Coder deployment's certificate cannot be verified because a certificate is missing from its chain. To fix this your deployment's administrator must bundle the missing certificates.", + // NON_SIGNING can be removed if BoringSSL is patched and the patch makes it + // into the version of Electron used by VS Code. + NON_SIGNING = "Your Coder deployment's certificate is not marked as being capable of signing. VS Code uses a version of Electron that does not support certificates like this even if they are self-issued. The certificate must be regenerated with the certificate signing capability.", + UNTRUSTED_LEAF = "Your Coder deployment's certificate does not appear to be trusted by this system. The certificate must be added to this system's trust store.", + 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. ", +} + +export class ServerCertificateError extends CertificateError { + public static readonly ActionAllowInsecure = "Allow Insecure"; + public static readonly InsecureMessage = + 'The Coder extension will no longer verify TLS on HTTPS requests. You can change this at any time with the "coder.insecure" property in your VS Code settings.'; + + private constructor( + message: string, + public readonly x509Err: X509_ERR, + ) { + super("Secure connection to your Coder deployment failed: " + message); + this.name = "ServerCertificateError"; + } + + get detail(): string { + return this.x509Err; + } + + // maybeWrap returns a ServerCertificateError if the code is a certificate error + // otherwise it returns the original error. + static async maybeWrap( + err: T, + address: string, + logger: Logger, + ): Promise { + if (isAxiosError(err)) { + switch (err.code) { + case X509_ERR_CODE.UNABLE_TO_VERIFY_LEAF_SIGNATURE: + // "Unable to verify" can mean different things so we will attempt to + // parse the certificate and determine which it is. + try { + const cause = + await ServerCertificateError.determineVerifyErrorCause(address); + return new ServerCertificateError(err.message, cause); + } catch (error) { + logger.warn(`Failed to parse certificate from ${address}`, error); + break; + } + case X509_ERR_CODE.DEPTH_ZERO_SELF_SIGNED_CERT: + return new ServerCertificateError( + err.message, + X509_ERR.UNTRUSTED_LEAF, + ); + case X509_ERR_CODE.SELF_SIGNED_CERT_IN_CHAIN: + return new ServerCertificateError( + err.message, + X509_ERR.UNTRUSTED_CHAIN, + ); + case undefined: + break; + } + } + return err; + } + + // determineVerifyErrorCause fetches the certificate(s) from the specified + // address, parses the leaf, and returns the reason the certificate is giving + // an "unable to verify" error or throws if unable to figure it out. + private static async determineVerifyErrorCause( + address: string, + ): Promise { + return new Promise((resolve, reject) => { + try { + const url = new URL(address); + const socket = tls.connect( + { + port: Number.parseInt(url.port, 10) || 443, + host: url.hostname, + rejectUnauthorized: false, + }, + () => { + const x509 = socket.getPeerX509Certificate(); + socket.destroy(); + if (!x509) { + throw new Error("no peer certificate"); + } + + // We use "@peculiar/x509" because Node's x509 returns an undefined `keyUsage`. + const cert = new X509Certificate(x509.toString()); + const isSelfIssued = cert.subject === cert.issuer; + if (!isSelfIssued) { + return resolve(X509_ERR.PARTIAL_CHAIN); + } + + // The key usage needs to exist but not have cert signing to fail. + 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", (err) => reject(toError(err))); + } catch (err) { + reject(toError(err)); + } + }); + } + + // allowInsecure updates the value of the "coder.insecure" property. + private allowInsecure(): void { + vscode.workspace + .getConfiguration() + .update("coder.insecure", true, vscode.ConfigurationTarget.Global); + vscode.window.showInformationMessage( + ServerCertificateError.InsecureMessage, + ); + } + + async showNotification( + title?: string, + options?: { modal?: boolean }, + ): Promise { + const val = await this.showErrorMessage( + title, + options, + // TODO: The insecure setting does not seem to work, even though it + // should, as proven by the tests. Even hardcoding rejectUnauthorized to + // false does not work; something seems to just be different when ran + // inside VS Code. Disabling the "Strict SSL" setting does not help + // either. For now avoid showing the button until this is sorted. + // ServerCertificateError.ActionAllowInsecure, + ); + switch (val) { + case ServerCertificateError.ActionAllowInsecure: + this.allowInsecure(); + break; + case undefined: + break; + } + } +} diff --git a/src/extension.ts b/src/extension.ts index 753b4500..bef6ec87 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -251,8 +251,8 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { } } catch (ex) { if (ex instanceof CertificateError) { - output.warn(ex.x509Err || ex.message); - await ex.showModal("Failed to open workspace"); + output.warn(ex.detail); + await ex.showNotification("Failed to open workspace", { modal: true }); } else if (isAxiosError(ex)) { const msg = getErrorMessage(ex, "None"); const detail = getErrorDetail(ex) || "None"; diff --git a/src/headers.ts b/src/headers.ts index d7007340..d7685b07 100644 --- a/src/headers.ts +++ b/src/headers.ts @@ -1,23 +1,11 @@ -import * as cp from "node:child_process"; import * as os from "node:os"; -import * as util from "node:util"; -import { toError } from "./error/errorUtils"; +import { execCommand } from "./command/exec"; import { type Logger } from "./logging/logger"; import { escapeCommandArg } from "./util"; import type { WorkspaceConfiguration } from "vscode"; -interface ExecException { - code?: number; - stderr?: string; - stdout?: string; -} - -function isExecException(err: unknown): err is ExecException { - return (err as ExecException).code !== undefined; -} - export function getHeaderCommand( config: Pick, ): string | undefined { @@ -66,25 +54,20 @@ export async function getHeaders( typeof command === "string" && command.trim().length > 0 ) { - let result: { stdout: string; stderr: string }; - try { - result = await util.promisify(cp.exec)(command, { - env: { - ...process.env, - CODER_URL: url, - }, - }); - } catch (error: unknown) { - if (isExecException(error)) { - logger.warn("Header command exited unexpectedly with code", error.code); - logger.warn("stdout:", error.stdout); - logger.warn("stderr:", error.stderr); - throw new Error( - `Header command exited unexpectedly with code ${error.code}`, - ); - } - const message = toError(error).message; - throw new Error(`Header command exited unexpectedly: ${message}`); + const result = await execCommand(command, logger, { + title: "Header command", + env: { + ...process.env, + CODER_URL: url, + }, + }); + + if (!result.success) { + throw new Error( + result.exitCode === undefined + ? "Header command exited unexpectedly" + : `Header command exited unexpectedly with code ${result.exitCode}`, + ); } if (!result.stdout) { // Allow no output for parity with the Coder CLI. diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index 4605a43f..1abb342c 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -290,8 +290,10 @@ export class LoginCoordinator implements vscode.Disposable { const message = getErrorMessage(err, "no response from the server"); if (isAutoLogin) { this.logger.warn("Failed to log in to Coder server:", message); + } else if (err instanceof CertificateError) { + void err.showNotification("Failed to log in to Coder server"); } else { - this.vscodeProposed.window.showErrorMessage( + void this.vscodeProposed.window.showErrorMessage( "Failed to log in to Coder server", { detail: message, @@ -338,7 +340,7 @@ export class LoginCoordinator implements vscode.Disposable { if (err instanceof CertificateError) { void err.showNotification(); return { - message: err.x509Err || err.message, + message: err.detail, severity: vscode.InputBoxValidationSeverity.Error, }; } diff --git a/src/websocket/reconnectingWebSocket.ts b/src/websocket/reconnectingWebSocket.ts index be6ffed4..5956da87 100644 --- a/src/websocket/reconnectingWebSocket.ts +++ b/src/websocket/reconnectingWebSocket.ts @@ -1,3 +1,4 @@ +import { ClientCertificateError } from "../error/clientCertificateError"; import { toError } from "../error/errorUtils"; import { @@ -22,6 +23,8 @@ export interface ReconnectingWebSocketOptions { initialBackoffMs?: number; maxBackoffMs?: number; jitterFactor?: number; + /** Callback invoked when a refreshable certificate error is detected. Returns true if refresh succeeded. */ + onCertificateRefreshNeeded: () => Promise; } export class ReconnectingWebSocket< @@ -47,12 +50,13 @@ export class ReconnectingWebSocket< #isDisposed = false; // Permanent disposal, cannot be resumed #isConnecting = false; #pendingReconnect = false; + #certRefreshAttempted = false; // Tracks if cert refresh was already attempted this connection cycle readonly #onDispose?: () => void; private constructor( socketFactory: SocketFactory, logger: Logger, - options: ReconnectingWebSocketOptions = {}, + options: ReconnectingWebSocketOptions, onDispose?: () => void, ) { this.#socketFactory = socketFactory; @@ -61,6 +65,7 @@ export class ReconnectingWebSocket< initialBackoffMs: options.initialBackoffMs ?? 250, maxBackoffMs: options.maxBackoffMs ?? 30000, jitterFactor: options.jitterFactor ?? 0.1, + onCertificateRefreshNeeded: options.onCertificateRefreshNeeded, }; this.#backoffMs = this.#options.initialBackoffMs; this.#onDispose = onDispose; @@ -69,7 +74,7 @@ export class ReconnectingWebSocket< static async create( socketFactory: SocketFactory, logger: Logger, - options: ReconnectingWebSocketOptions = {}, + options: ReconnectingWebSocketOptions, onDispose?: () => void, ): Promise> { const instance = new ReconnectingWebSocket( @@ -78,6 +83,8 @@ export class ReconnectingWebSocket< options, onDispose, ); + + // connect() handles all errors internally await instance.connect(); return instance; } @@ -86,6 +93,14 @@ export class ReconnectingWebSocket< return this.#currentSocket?.url ?? ""; } + /** + * Returns true if the socket is temporarily disconnected and not attempting to reconnect. + * Use reconnect() to resume. + */ + get isDisconnected(): boolean { + return this.#isDisconnected; + } + /** * Extract the route (pathname + search) from the current socket URL for logging. * Falls back to the last known route when the socket is closed. @@ -121,6 +136,7 @@ export class ReconnectingWebSocket< if (this.#isDisconnected) { this.#isDisconnected = false; this.#backoffMs = this.#options.initialBackoffMs; + this.#certRefreshAttempted = false; // User-initiated reconnect, allow retry } if (this.#isDisposed) { @@ -132,14 +148,13 @@ export class ReconnectingWebSocket< this.#reconnectTimeoutId = null; } - // If already connecting, schedule reconnect after current attempt if (this.#isConnecting) { this.#pendingReconnect = true; return; } - // connect() will close any existing socket - this.connect().catch((error) => this.handleConnectionError(error)); + // connect() handles all errors internally + void this.connect(); } /** @@ -200,6 +215,7 @@ export class ReconnectingWebSocket< socket.addEventListener("open", (event) => { this.#backoffMs = this.#options.initialBackoffMs; + this.#certRefreshAttempted = false; // Reset on successful connection this.executeHandlers("open", event); }); @@ -209,17 +225,11 @@ export class ReconnectingWebSocket< socket.addEventListener("error", (event) => { this.executeHandlers("error", event); - - // Check for unrecoverable HTTP errors in the error event - // HTTP errors during handshake fire 'error' then 'close' with 1006 - // We need to suspend here to prevent infinite reconnect loops - const errorMessage = toError(event.error, event.message).message; - if (this.isUnrecoverableHttpError(errorMessage)) { - this.#logger.error( - `Unrecoverable HTTP error for ${this.#route}: ${errorMessage}`, - ); - this.disconnect(); - } + // Errors during initial connection are caught by the factory (waitForOpen). + // This handler is for errors AFTER successful connection. + // Route through handleConnectionError for consistent handling. + const error = toError(event.error, event.message); + void this.handleConnectionError(error); }); socket.addEventListener("close", (event) => { @@ -233,19 +243,18 @@ export class ReconnectingWebSocket< this.#logger.error( `WebSocket connection closed with unrecoverable error code ${event.code}`, ); - // Suspend instead of dispose - allows recovery when credentials change this.disconnect(); return; } - // Don't reconnect on normal closure if (NORMAL_CLOSURE_CODES.has(event.code)) { return; } - // Reconnect on abnormal closures (e.g., 1006) or other unexpected codes this.scheduleReconnect(); }); + } catch (error) { + await this.handleConnectionError(error); } finally { this.#isConnecting = false; @@ -275,12 +284,55 @@ export class ReconnectingWebSocket< this.#reconnectTimeoutId = setTimeout(() => { this.#reconnectTimeoutId = null; - this.connect().catch((error) => this.handleConnectionError(error)); + // connect() handles all errors internally + void this.connect(); }, delayMs); this.#backoffMs = Math.min(this.#backoffMs * 2, this.#options.maxBackoffMs); } + /** + * Attempt to refresh certificates and return true if refresh succeeded. + */ + private async attemptCertificateRefresh(): Promise { + try { + return await this.#options.onCertificateRefreshNeeded(); + } catch (refreshError) { + this.#logger.error("Error during certificate refresh:", refreshError); + return false; + } + } + + /** + * Handle client certificate errors by attempting refresh for refreshable errors. + * @returns true if refresh succeeded. + */ + private async handleClientCertificateError( + certError: ClientCertificateError, + ): Promise { + // Only attempt refresh once per connection cycle + if (this.#certRefreshAttempted) { + this.#logger.warn("Certificate refresh already attempted, not retrying"); + void certError.showNotification(); + return false; + } + + if (certError.isRefreshable) { + this.#certRefreshAttempted = true; // Mark that we're attempting + this.#logger.info( + `Client certificate error (alert ${certError.alertCode}), attempting refresh...`, + ); + if (await this.attemptCertificateRefresh()) { + this.#logger.info("Certificate refresh succeeded, reconnecting..."); + return true; + } + } + + // Show notification for failed/non-refreshable errors + void certError.showNotification(); + return false; + } + private executeHandlers( event: TEvent, eventData: Parameters>[0], @@ -301,7 +353,7 @@ export class ReconnectingWebSocket< * Checks if the error is unrecoverable and suspends the connection, * otherwise schedules a reconnect. */ - private handleConnectionError(error: unknown): void { + private async handleConnectionError(error: unknown): Promise { if (this.#isDisposed || this.#isDisconnected) { return; } @@ -315,6 +367,17 @@ export class ReconnectingWebSocket< return; } + // Check for certificate error and attempt refresh if possible. + const certError = ClientCertificateError.fromError(error); + if (certError) { + if (await this.handleClientCertificateError(certError)) { + this.reconnect(); + } else { + this.disconnect(); + } + return; + } + this.#logger.warn(`WebSocket connection failed for ${this.#route}`, error); this.scheduleReconnect(); } diff --git a/test/unit/api/certificateRefresh.test.ts b/test/unit/api/certificateRefresh.test.ts new file mode 100644 index 00000000..3b4c8ea9 --- /dev/null +++ b/test/unit/api/certificateRefresh.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; + +import { refreshCertificates } from "@/api/certificateRefresh"; + +import { createMockLogger } from "../../mocks/testHelpers"; +import { exitCommand, printCommand } from "../../utils/platform"; + +const logger = createMockLogger(); + +describe("refreshCertificates", () => { + it("should return true on successful command", async () => { + const result = await refreshCertificates( + printCommand("certificates refreshed"), + logger, + ); + expect(result).toBe(true); + }); + + it("should return false on command failure", async () => { + const result = await refreshCertificates(exitCommand(1), logger); + expect(result).toBe(false); + }); + + it("should return false on non-existent command", async () => { + const result = await refreshCertificates( + "nonexistent-command-that-should-not-exist", + logger, + ); + expect(result).toBe(false); + }); +}); diff --git a/test/unit/api/coderApi.test.ts b/test/unit/api/coderApi.test.ts index 9cb4a108..b8400400 100644 --- a/test/unit/api/coderApi.test.ts +++ b/test/unit/api/coderApi.test.ts @@ -12,7 +12,7 @@ import Ws from "ws"; import { CoderApi } from "@/api/coderApi"; import { createHttpAgent } from "@/api/utils"; -import { CertificateError } from "@/error/certificateError"; +import { ServerCertificateError } from "@/error/serverCertificateError"; import { getHeaders } from "@/headers"; import { type RequestConfigWithMeta } from "@/logging/types"; import { ReconnectingWebSocket } from "@/websocket/reconnectingWebSocket"; @@ -133,9 +133,9 @@ describe("CoderApi", () => { .get("/api/v2/users/me") .catch((e) => e); - expect(thrownError).toBeInstanceOf(CertificateError); + expect(thrownError).toBeInstanceOf(ServerCertificateError); - const castError = thrownError as CertificateError; + const castError = thrownError as ServerCertificateError; expect(castError.message).toContain("Secure connection"); expect(castError.x509Err).toBeDefined(); }); @@ -217,7 +217,15 @@ describe("CoderApi", () => { beforeEach(() => { api = createApi(CODER_URL, AXIOS_TOKEN); - const mockWs = createMockWebSocket(wsUrl); + // createOneWayWebSocket waits for open, so we need to fire it + const mockWs = createMockWebSocket(wsUrl, { + on: vi.fn((event, handler) => { + if (event === "open") { + setImmediate(() => handler()); + } + return mockWs as Ws; + }), + }); setupWebSocketMock(mockWs); }); @@ -390,15 +398,18 @@ describe("CoderApi", () => { expect(EventSource).toHaveBeenCalled(); }); - it("throws non-404 errors without SSE fallback", async () => { + it("handles non-404 errors without SSE fallback", async () => { vi.mocked(Ws).mockImplementation(function () { throw new Error("Network error"); }); - await expect(api.watchAgentMetadata(AGENT_ID)).rejects.toThrow( - "Network error", - ); + // Non-404 errors are handled by ReconnectingWebSocket internally + // (schedules reconnect with backoff) instead of being thrown + const connection = await api.watchAgentMetadata(AGENT_ID); + expect(connection).toBeInstanceOf(ReconnectingWebSocket); expect(EventSource).not.toHaveBeenCalled(); + + connection.close(); }); describe("reconnection after fallback", () => { @@ -572,6 +583,45 @@ describe("CoderApi", () => { "No base URL set on REST client", ); }); + + interface WebSocketErrorConversionTestCase { + description: string; + eventMessage: string; + expectedMessage: string; + } + + it.each([ + { + description: "event.message when available", + eventMessage: "Custom error message", + expectedMessage: "Custom error message", + }, + { + description: "fallback when event.message is empty", + eventMessage: "", + expectedMessage: "WebSocket connection error", + }, + ])( + "uses $desc for WebSocket error", + async ({ eventMessage, expectedMessage }) => { + api = createApi(); + const mockWs = createMockWebSocket("wss://test", { + on: vi.fn((event: string, handler: (e: unknown) => void) => { + if (event === "error") { + setImmediate(() => + handler({ error: undefined, message: eventMessage }), + ); + } + return mockWs as Ws; + }), + }); + setupWebSocketMock(mockWs); + + await expect(api.watchBuildLogsByBuildId(BUILD_ID, [])).rejects.toThrow( + expectedMessage, + ); + }, + ); }); describe("getHost/getSessionToken", () => { diff --git a/test/unit/command/exec.test.ts b/test/unit/command/exec.test.ts new file mode 100644 index 00000000..3041ebf2 --- /dev/null +++ b/test/unit/command/exec.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; + +import { execCommand } from "@/command/exec"; + +import { createMockLogger } from "../../mocks/testHelpers"; +import { + exitCommand, + printCommand, + printEnvCommand, +} from "../../utils/platform"; + +const logger = createMockLogger(); + +describe("execCommand", () => { + it("should return success with stdout on successful command", async () => { + const result = await execCommand(printCommand("hello"), logger, { + title: "Test", + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.stdout).toContain("hello"); + } + }); + + it("should return failure with exit code on command failure", async () => { + const result = await execCommand(exitCommand(42), logger, { + title: "Test", + }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.exitCode).toBe(42); + } + }); + + it("should return failure for non-existent command", async () => { + const result = await execCommand("nonexistent-cmd-12345", logger, { + title: "Test", + }); + expect(result.success).toBe(false); + }); + + it("should pass environment variables to command", async () => { + const result = await execCommand( + printEnvCommand("TEST_VAR", "TEST_VAR"), + logger, + { + title: "Test", + env: { ...process.env, TEST_VAR: "test_value" }, + }, + ); + expect(result.success).toBe(true); + if (result.success) { + expect(result.stdout).toContain("test_value"); + } + }); + + it("should use default title when not provided", async () => { + const result = await execCommand(printCommand("test"), logger); + expect(result.success).toBe(true); + }); +}); diff --git a/test/unit/error/clientCertificateError.test.ts b/test/unit/error/clientCertificateError.test.ts new file mode 100644 index 00000000..9271a644 --- /dev/null +++ b/test/unit/error/clientCertificateError.test.ts @@ -0,0 +1,177 @@ +import { describe, expect, it } from "vitest"; + +import { + ClientCertificateError, + CLIENT_CERT_ALERT, + CLIENT_CERT_MESSAGES, +} from "@/error/clientCertificateError"; + +type AlertName = keyof typeof CLIENT_CERT_ALERT; +type AlertTestCase = [AlertName, string]; +type LowercaseTestCase = [string, CLIENT_CERT_ALERT]; +type AlertNumberTestCase = [number, CLIENT_CERT_ALERT]; + +describe("ClientCertificateError.fromError", () => { + describe("SSLV3_ALERT_* patterns", () => { + it.each([ + ["BAD_CERTIFICATE", "SSLV3_ALERT_BAD_CERTIFICATE:SSL alert number 42"], + [ + "UNSUPPORTED_CERTIFICATE", + "SSLV3_ALERT_UNSUPPORTED_CERTIFICATE:SSL alert number 43", + ], + [ + "CERTIFICATE_REVOKED", + "SSLV3_ALERT_CERTIFICATE_REVOKED:SSL alert number 44", + ], + [ + "CERTIFICATE_EXPIRED", + "write EPROTO 18468360202752:error:10000415:SSL routines:OPENSSL_internal:SSLV3_ALERT_CERTIFICATE_EXPIRED:../../third_party/boringssl/src/ssl/tls_record.cc:486:SSL alert number 45", + ], + [ + "CERTIFICATE_UNKNOWN", + "SSLV3_ALERT_CERTIFICATE_UNKNOWN:SSL alert number 46", + ], + ["UNKNOWN_CA", "SSLV3_ALERT_UNKNOWN_CA:SSL alert number 48"], + ["ACCESS_DENIED", "SSLV3_ALERT_ACCESS_DENIED:SSL alert number 49"], + ])("should detect %s", (alertName, message) => { + const certError = ClientCertificateError.fromError(new Error(message)); + expect(certError).toBeDefined(); + expect(certError!.alertCode).toBe(CLIENT_CERT_ALERT[alertName]); + }); + }); + + describe("lowercase patterns", () => { + it.each([ + ["certificate_expired", CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED], + ["bad_certificate", CLIENT_CERT_ALERT.BAD_CERTIFICATE], + ["unknown_ca", CLIENT_CERT_ALERT.UNKNOWN_CA], + ])("should detect %s", (pattern, expectedCode) => { + const certError = ClientCertificateError.fromError( + new Error(`SSL error: ${pattern}`), + ); + expect(certError).toBeDefined(); + expect(certError!.alertCode).toBe(expectedCode); + }); + }); + + describe("SSL alert number fallback", () => { + it.each([ + [45, CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED], + [42, CLIENT_CERT_ALERT.BAD_CERTIFICATE], + [48, CLIENT_CERT_ALERT.UNKNOWN_CA], + ])("should detect alert number %i", (alertNumber, expectedCode) => { + const certError = ClientCertificateError.fromError( + new Error(`SSL handshake failed: SSL alert number ${alertNumber}`), + ); + expect(certError).toBeDefined(); + expect(certError!.alertCode).toBe(expectedCode); + }); + + it("should return undefined for unknown alert numbers", () => { + const certError = ClientCertificateError.fromError( + new Error("SSL handshake failed: SSL alert number 99"), + ); + expect(certError).toBeUndefined(); + }); + }); + + describe("error types", () => { + it("should handle plain Error objects", () => { + const certError = ClientCertificateError.fromError( + new Error("SSLV3_ALERT_CERTIFICATE_EXPIRED"), + ); + expect(certError).toBeDefined(); + expect(certError!.alertCode).toBe(CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED); + }); + + it("should handle string errors", () => { + const certError = ClientCertificateError.fromError( + "SSLV3_ALERT_CERTIFICATE_EXPIRED", + ); + expect(certError).toBeDefined(); + expect(certError!.alertCode).toBe(CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED); + + expect( + ClientCertificateError.fromError("some other error"), + ).toBeUndefined(); + }); + + it("should handle errors with code property", () => { + const certError = ClientCertificateError.fromError({ + code: "ERR_SSL_SSLV3_ALERT_CERTIFICATE_EXPIRED", + message: "certificate has expired", + }); + expect(certError).toBeDefined(); + expect(certError!.alertCode).toBe(CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED); + }); + + it.each([null, undefined])("should return undefined for %s", (value) => { + expect(ClientCertificateError.fromError(value)).toBeUndefined(); + }); + + it.each([ + ["non-SSL errors", "Connection refused"], + ["other SSL errors", "SSL error: UNABLE_TO_VERIFY_LEAF_SIGNATURE"], + ])("should return undefined for %s", (_desc, message) => { + expect( + ClientCertificateError.fromError(new Error(message)), + ).toBeUndefined(); + }); + }); +}); + +describe("ClientCertificateError.isRefreshable", () => { + it.each([ + ["CERTIFICATE_EXPIRED", "SSLV3_ALERT_CERTIFICATE_EXPIRED"], + ["CERTIFICATE_REVOKED", "SSLV3_ALERT_CERTIFICATE_REVOKED"], + ["BAD_CERTIFICATE", "SSLV3_ALERT_BAD_CERTIFICATE"], + ["CERTIFICATE_UNKNOWN", "SSLV3_ALERT_CERTIFICATE_UNKNOWN"], + ])("should return true for refreshable alert %s", (_name, msg) => { + const error = new Error(msg); + expect(ClientCertificateError.isRefreshable(error)).toBe(true); + }); + + it.each([ + ["UNKNOWN_CA", "SSLV3_ALERT_UNKNOWN_CA"], + ["ACCESS_DENIED", "SSLV3_ALERT_ACCESS_DENIED"], + ["UNSUPPORTED_CERTIFICATE", "SSLV3_ALERT_UNSUPPORTED_CERTIFICATE"], + ])("should return false for non-refreshable alert %s", (_name, msg) => { + const error = new Error(msg); + expect(ClientCertificateError.isRefreshable(error)).toBe(false); + }); + + it("should return false for non-certificate errors", () => { + expect( + ClientCertificateError.isRefreshable(new Error("Connection refused")), + ).toBe(false); + expect(ClientCertificateError.isRefreshable(null)).toBe(false); + expect(ClientCertificateError.isRefreshable(undefined)).toBe(false); + }); +}); + +describe("ClientCertificateError properties", () => { + it("should be refreshable with guidance for refreshable errors", () => { + const certError = ClientCertificateError.fromError( + new Error("SSLV3_ALERT_CERTIFICATE_EXPIRED"), + ); + expect(certError).toBeDefined(); + expect(certError!.isRefreshable).toBe(true); + expect(certError!.alertCode).toBe(CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED); + expect(certError!.detail).toContain( + CLIENT_CERT_MESSAGES[CLIENT_CERT_ALERT.CERTIFICATE_EXPIRED], + ); + expect(certError!.detail).toContain("Try refreshing your credentials"); + }); + + it("should not be refreshable for configuration errors", () => { + const certError = ClientCertificateError.fromError( + new Error("SSLV3_ALERT_UNKNOWN_CA"), + ); + expect(certError).toBeDefined(); + expect(certError!.isRefreshable).toBe(false); + expect(certError!.detail).toContain( + CLIENT_CERT_MESSAGES[CLIENT_CERT_ALERT.UNKNOWN_CA], + ); + expect(certError!.detail).toContain("cannot be resolved by refreshing"); + }); +}); diff --git a/test/unit/error/certificateError.test.ts b/test/unit/error/serverCertificateError.test.ts similarity index 89% rename from test/unit/error/certificateError.test.ts rename to test/unit/error/serverCertificateError.test.ts index 9fd374e5..0265bd62 100644 --- a/test/unit/error/certificateError.test.ts +++ b/test/unit/error/serverCertificateError.test.ts @@ -8,11 +8,12 @@ import * as fs from "node:fs/promises"; import https from "node:https"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { CertificateError } from "@/error/certificateError"; import { - CertificateError, + ServerCertificateError, X509_ERR, X509_ERR_CODE, -} from "@/error/certificateError"; +} from "@/error/serverCertificateError"; import { type Logger } from "@/logging/logger"; import { getFixturePath } from "../../utils/fixtures"; @@ -101,9 +102,13 @@ describe("Certificate errors", () => { try { await request; } catch (error) { - const wrapped = await CertificateError.maybeWrap(error, address, logger); + const wrapped = await ServerCertificateError.maybeWrap( + error, + address, + logger, + ); expect(wrapped instanceof CertificateError).toBeTruthy(); - expect((wrapped as CertificateError).x509Err).toBe( + expect((wrapped as ServerCertificateError).x509Err).toBe( X509_ERR.PARTIAL_CHAIN, ); } @@ -136,9 +141,15 @@ describe("Certificate errors", () => { try { await request; } catch (error) { - const wrapped = await CertificateError.maybeWrap(error, address, logger); + const wrapped = await ServerCertificateError.maybeWrap( + error, + address, + logger, + ); expect(wrapped instanceof CertificateError).toBeTruthy(); - expect((wrapped as CertificateError).x509Err).toBe(X509_ERR.NON_SIGNING); + expect((wrapped as ServerCertificateError).x509Err).toBe( + X509_ERR.NON_SIGNING, + ); } }); @@ -182,9 +193,13 @@ describe("Certificate errors", () => { try { await request; } catch (error) { - const wrapped = await CertificateError.maybeWrap(error, address, logger); + const wrapped = await ServerCertificateError.maybeWrap( + error, + address, + logger, + ); expect(wrapped instanceof CertificateError).toBeTruthy(); - expect((wrapped as CertificateError).x509Err).toBe( + expect((wrapped as ServerCertificateError).x509Err).toBe( X509_ERR.UNTRUSTED_LEAF, ); } @@ -225,9 +240,13 @@ describe("Certificate errors", () => { try { await request; } catch (error) { - const wrapped = await CertificateError.maybeWrap(error, address, logger); + const wrapped = await ServerCertificateError.maybeWrap( + error, + address, + logger, + ); expect(wrapped instanceof CertificateError).toBeTruthy(); - expect((wrapped as CertificateError).x509Err).toBe( + expect((wrapped as ServerCertificateError).x509Err).toBe( X509_ERR.UNTRUSTED_CHAIN, ); } @@ -268,7 +287,11 @@ describe("Certificate errors", () => { try { await request; } catch (error) { - const wrapped = await CertificateError.maybeWrap(error, "1", logger); + const wrapped = await ServerCertificateError.maybeWrap( + error, + "1", + logger, + ); expect(wrapped instanceof CertificateError).toBeFalsy(); expect((wrapped as Error).message).toMatch(/failed with status code 500/); } diff --git a/test/unit/headers.test.ts b/test/unit/headers.test.ts index f5812ec1..15b09117 100644 --- a/test/unit/headers.test.ts +++ b/test/unit/headers.test.ts @@ -2,17 +2,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { type WorkspaceConfiguration } from "vscode"; import { getHeaderCommand, getHeaders } from "@/headers"; -import { type Logger } from "@/logging/logger"; +import { createMockLogger } from "../mocks/testHelpers"; import { printCommand, exitCommand, printEnvCommand } from "../utils/platform"; -const logger: Logger = { - trace: () => {}, - debug: () => {}, - info: () => {}, - warn: () => {}, - error: () => {}, -}; +const logger = createMockLogger(); describe("Headers", () => { it("should return no headers", async () => { diff --git a/test/unit/websocket/reconnectingWebSocket.test.ts b/test/unit/websocket/reconnectingWebSocket.test.ts index bfdc4012..ec78bcbb 100644 --- a/test/unit/websocket/reconnectingWebSocket.test.ts +++ b/test/unit/websocket/reconnectingWebSocket.test.ts @@ -90,13 +90,21 @@ describe("ReconnectingWebSocket", () => { ); }); - await expect( - ReconnectingWebSocket.create(factory, createMockLogger()), - ).rejects.toThrow(`Unexpected server response: ${statusCode}`); + // create() returns a disconnected instance instead of throwing + const ws = await ReconnectingWebSocket.create( + factory, + createMockLogger(), + { onCertificateRefreshNeeded: () => Promise.resolve(false) }, + ); + + // Should be disconnected after unrecoverable HTTP error + expect(ws.isDisconnected).toBe(true); // Should not retry after unrecoverable HTTP error await vi.advanceTimersByTimeAsync(10000); expect(socketCreationAttempts).toBe(1); + + ws.close(); }, ); @@ -620,7 +628,7 @@ async function fromFactory( return await ReconnectingWebSocket.create( factory, createMockLogger(), - undefined, + { onCertificateRefreshNeeded: () => Promise.resolve(false) }, onDispose, ); }