Improve credential lifecycle and log hygiene

src/command/exec.ts

@@ -33,19 +33,14 @@ export async function execCommand(
 	options?: ExecCommandOptions,
 ): Promise<ExecCommandResult> {
 	const title = options?.title ?? "Command";
-	logger.debug(`Executing ${title}: ${command}`);
+	// The command string and its output can carry credentials, so log neither.
+	logger.debug(`Executing ${title}`);
 
 	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,
@@ -54,12 +49,6 @@ export async function execCommand(
 	} 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,
@@ -68,7 +57,7 @@ export async function execCommand(
 			};
 		}
 
-		logger.warn(`${title} failed:`, error);
+		logger.warn(`${title} failed to execute`);
 		return { success: false };
 	}
 }

src/commands.ts

@@ -434,6 +434,11 @@ export class Commands {
 
 		const { agentName, client, workspaceId, remoteAuthority } = resolved;
 
+		if (!(await this.confirmSupportBundleCollection())) {
+			telemetry.abort("prompt");
+			return;
+		}
+
 		const outputUri = await this.promptSupportBundlePath();
 		if (!outputUri) {
 			telemetry.abort("save_dialog");
@@ -523,6 +528,27 @@ export class Commands {
 		});
 	}
 
+	/** Modal disclosure of what a support bundle collects; the CLI's own prompt is suppressed. */
+	private async confirmSupportBundleCollection(): Promise<boolean> {
+		const detail = [
+			"A support bundle may contain sensitive information. It collects:",
+			"",
+			"\u2022 Deployment and workspace diagnostics",
+			"\u2022 Coder extension and connection logs from recent VS Code windows",
+			"\u2022 Remote SSH extension logs",
+			"\u2022 Locally recorded telemetry",
+			"\u2022 Coder extension settings",
+			"",
+			"Review the bundle before sharing it.",
+		].join("\n");
+		const choice = await vscode.window.showInformationMessage(
+			"Create a support bundle?",
+			{ modal: true, detail },
+			"Continue",
+		);
+		return choice === "Continue";
+	}
+
 	public async exportTelemetry(): Promise<void> {
 		await this.diagnosticTelemetry.trace("export_telemetry", (telemetry) =>
 			this.runExportTelemetry(telemetry),
@@ -596,8 +622,14 @@ export class Commands {
 		await this.deploymentManager.clearDeployment("logout");
 
 		if (deployment) {
-			await this.cliManager.clearCredentials(deployment.url);
+			const cleared = await this.cliManager.clearCredentials(deployment.url);
 			await this.secretsManager.clearAllAuthData(deployment.safeHostname);
+			if (!cleared) {
+				vscode.window.showWarningMessage(
+					'You\'ve been logged out of Coder, but some credentials could not be removed. Log out again to retry, or run "coder logout" in a terminal.',
+				);
+				return { success: false, reason: "cleanup_incomplete" };
+			}
 		}
 
 		this.showLogoutMessage();

src/core/cliCredentialManager.ts

@@ -171,39 +171,42 @@ export class CliCredentialManager {
 
 	/**
 	 * Delete credentials for a deployment. Removes the default-dir files and
-	 * logs out of the active store (keyring or file via --global-config), both
-	 * best-effort. Throws AbortError when the signal is aborted.
+	 * logs out of the active store (keyring or file via --global-config).
+	 * Returns whether every store was cleared instead of throwing, except
+	 * for AbortError when the signal is aborted.
 	 */
 	public deleteToken(
 		url: string,
 		configs: Pick<WorkspaceConfiguration, "get">,
 		options?: { signal?: AbortSignal },
-	): Promise<void> {
+	): Promise<boolean> {
 		return this.credentialTelemetry.traceClear(configs, async (span) => {
-			await Promise.all([
+			const [filesCleared, cliCleared] = await Promise.all([
 				this.deleteCredentialFiles(url),
 				this.cliLogout(url, configs, { signal: options?.signal, span }),
 			]);
+			return filesCleared && cliCleared;
 		});
 	}
 
 	/**
 	 * Log out via `coder logout`, keyring or file (--global-config). Records
-	 * failures on the span instead of throwing (except on abort).
+	 * failures on the span instead of throwing (except on abort) and returns
+	 * whether the logout succeeded.
 	 */
 	private async cliLogout(
 		url: string,
 		configs: Pick<WorkspaceConfiguration, "get">,
 		{ signal, span }: { signal?: AbortSignal; span: Span },
-	): Promise<void> {
+	): Promise<boolean> {
 		let transport: CliTransport;
 		try {
 			transport = await this.resolveWriteTransport(url, configs);
 		} catch (error) {
 			this.logger.warn("Could not resolve CLI binary for logout:", error);
 			span.setProperty("error.type", "binary");
 			span.markError();
-			return;
+			return false;
 		}
 		const args = [
 			...this.credentialGlobalFlags(transport, url, configs),
@@ -215,13 +218,15 @@ export class CliCredentialManager {
 		try {
 			await this.execWithTimeout(transport.binPath, args, { signal });
 			this.logger.info("Deleted token via CLI for", url);
+			return true;
 		} catch (error) {
 			if (isAbortError(error)) {
 				throw error;
 			}
 			this.logger.warn("Failed to delete token via CLI:", error);
 			span.setProperty("error.type", "cli");
 			span.markError();
+			return false;
 		}
 	}
 
@@ -311,21 +316,27 @@ export class CliCredentialManager {
 	}
 
 	/**
-	 * Delete URL and token files. Best-effort: never throws.
+	 * Delete URL and token files. Returns whether all removals succeeded;
+	 * never throws.
 	 */
-	private async deleteCredentialFiles(url: string): Promise<void> {
+	private async deleteCredentialFiles(url: string): Promise<boolean> {
 		const safeHostname = toSafeHost(url);
 		const paths = [
 			this.pathResolver.getSessionTokenPath(safeHostname),
 			this.pathResolver.getUrlPath(safeHostname),
 		];
-		await Promise.all(
+		const results = await Promise.all(
 			paths.map((p) =>
-				fs.rm(p, { force: true }).catch((error) => {
-					this.logger.warn("Failed to remove credential file", p, error);
-				}),
+				fs.rm(p, { force: true }).then(
+					() => true,
+					(error) => {
+						this.logger.warn("Failed to remove credential file", p, error);
+						return false;
+					},
+				),
 			),
 		);
+		return results.every(Boolean);
 	}
 }
 

src/core/cliManager.ts

@@ -1061,9 +1061,10 @@ export class CliManager {
 
 	/**
 	 * Remove credentials for a deployment. Clears both file-based credentials
-	 * and keyring entries (via `coder logout`). All cleanup is best-effort.
+	 * and keyring entries (via `coder logout`). Never throws; returns whether
+	 * every store was cleared.
 	 */
-	public async clearCredentials(url: string): Promise<void> {
+	public async clearCredentials(url: string): Promise<boolean> {
 		const configs = vscode.workspace.getConfiguration();
 		const result = await withOptionalProgress(
 			({ signal }) =>
@@ -1076,13 +1077,14 @@ export class CliManager {
 			},
 		);
 		if (result.ok) {
-			return;
+			return result.value;
 		}
 		if (result.cancelled) {
 			this.output.info("Credential removal cancelled by user");
 		} else {
 			this.output.warn("Failed to remove credentials:", result.error);
 		}
+		return false;
 	}
 
 	private handleStoreError(error: unknown): void {

src/deployment/deploymentManager.ts

@@ -201,6 +201,10 @@ export class DeploymentManager implements vscode.Disposable {
 			"Clearing deployment",
 			this.#sessionStore.current.deployment?.safeHostname,
 		);
+		if (reason === "logout") {
+			// Best-effort server-side revocation before local state is cleared.
+			await this.oauthSessionManager.revokeTokens();
+		}
 		const wasAuthenticated = this.isAuthenticated();
 		this.#authListenerDisposable?.dispose();
 		this.#authListenerDisposable = undefined;

src/headers.ts

@@ -38,13 +38,14 @@ export async function getHeaders(
 			return headers;
 		}
 		const lines = result.stdout.replace(/\r?\n$/, "").split(/\r?\n/);
-		for (const line of lines) {
+		for (const [index, line] of lines.entries()) {
 			const [key, value] = line.split(/=(.*)/);
 			// Header names cannot be blank or contain whitespace and the Coder CLI
 			// requires that there be an equals sign (the value can be blank though).
 			if (key.length === 0 || key.includes(" ") || value === undefined) {
+				// The output can carry credentials; reference the line by number only.
 				throw new Error(
-					`Malformed line from header command: [${line}] (out: ${result.stdout})`,
+					`Malformed line ${index + 1} from header command output`,
 				);
 			}
 			headers[key] = value;

src/instrumentation/auth.ts

@@ -17,7 +17,8 @@ export type AuthLoginOutcome =
 	| { success: true; method: LoginMethod }
 	| { success: false; method?: LoginMethod; reason: LoginPromptReason };
 export type AuthLogoutOutcome =
-	{ success: true } | { success: false; reason: "not_authenticated" };
+	| { success: true }
+	| { success: false; reason: "not_authenticated" | "cleanup_incomplete" };
 
 interface AuthLoginTrace {
 	setMethod: (method: LoginMethod) => void;

src/instrumentation/credentials.ts

@@ -27,25 +27,26 @@ export class CredentialTelemetry {
 		return this.trace("auth.credential.store", configs, fn);
 	}
 
-	public traceClear(
+	public traceClear<T>(
 		configs: Pick<WorkspaceConfiguration, "get">,
-		fn: (span: Span) => Promise<void>,
-	): Promise<void> {
+		fn: (span: Span) => Promise<T>,
+	): Promise<T> {
 		return this.trace("auth.credential.clear", configs, fn);
 	}
 
-	private async trace(
+	private async trace<T>(
 		eventName: CredentialEvent,
 		configs: Pick<WorkspaceConfiguration, "get">,
-		fn: (span: Span) => Promise<void>,
-	): Promise<void> {
+		fn: (span: Span) => Promise<T>,
+	): Promise<T> {
 		const keyringEnabled = isKeyringEnabled(configs);
 		let aborted: Error | undefined;
+		let result: T | undefined;
 		await this.telemetry.trace(
 			eventName,
 			async (span) => {
 				try {
-					await fn(span);
+					result = await fn(span);
 				} catch (error) {
 					if (isAbortError(error)) {
 						span.markAborted();
@@ -64,6 +65,7 @@ export class CredentialTelemetry {
 		if (aborted) {
 			throw aborted;
 		}
+		return result as T;
 	}
 }