diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index be033689..7d8d66cf 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -8,18 +8,16 @@ import type { Memento, SecretStorage, Disposable } from "vscode"; import type { Logger } from "../logging/logger"; -// Each deployment has its own key to ensure atomic operations (multiple windows -// writing to a shared key could drop data) and to receive proper VS Code events. +// Per-deployment keys ensure atomic operations (multiple windows writing to a +// shared key could drop data) and enable proper VS Code change events. const SESSION_KEY_PREFIX = "coder.session."; const OAUTH_CLIENT_PREFIX = "coder.oauth.client."; +const DEPLOYMENT_ACCESS_PREFIX = "coder.access."; type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX; const OAUTH_CALLBACK_KEY = "coder.oauthCallback"; - const CURRENT_DEPLOYMENT_KEY = "coder.currentDeployment"; - -const DEPLOYMENT_USAGE_KEY = "coder.deploymentUsage"; const DEFAULT_MAX_DEPLOYMENTS = 10; const LEGACY_SESSION_TOKEN_KEY = "sessionToken"; @@ -53,14 +51,6 @@ const SessionAuthSchema = z.object({ export type SessionAuth = z.infer; -// Tracks when a deployment was last accessed for LRU pruning. -const DeploymentUsageSchema = z.object({ - safeHostname: z.string(), - lastAccessedAt: z.string(), -}); - -type DeploymentUsage = z.infer; - const OAuthCallbackDataSchema = z.object({ state: z.string(), code: z.string().nullable(), @@ -224,61 +214,63 @@ export class SecretsManager { } /** - * Record that a deployment was accessed, moving it to the front of the LRU list. + * Record that a deployment was accessed by updating its timestamp. * Prunes deployments beyond maxCount, clearing their auth data. */ public async recordDeploymentAccess( safeHostname: string, maxCount = DEFAULT_MAX_DEPLOYMENTS, ): Promise { - const usage = this.getDeploymentUsage(); - const filtered = usage.filter((u) => u.safeHostname !== safeHostname); - const newEntry = DeploymentUsageSchema.parse({ - safeHostname, - lastAccessedAt: new Date().toISOString(), - }); - filtered.unshift(newEntry); - - const toKeep = filtered.slice(0, maxCount); - const toRemove = filtered.slice(maxCount); - - await Promise.all( - toRemove.map((u) => this.clearAllAuthData(u.safeHostname)), + // Update this deployment's access timestamp + await this.memento.update( + `${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`, + new Date().toISOString(), ); - await this.memento.update(DEPLOYMENT_USAGE_KEY, toKeep); + + // Prune if needed - errors here shouldn't block deployment access + try { + const allHostnames = await this.getKnownSafeHostnames(); + const toRemove = allHostnames.slice(maxCount); + await Promise.all(toRemove.map((h) => this.clearAllAuthData(h))); + } catch { + this.logger.warn("Failed to prune old deployments"); + } } /** - * Clear all auth data for a deployment and remove it from the usage list. + * Clear all auth data for a deployment, including the access timestamp. */ public async clearAllAuthData(safeHostname: string): Promise { await Promise.all([ this.clearSessionAuth(safeHostname), this.clearOAuthClientRegistration(safeHostname), + this.memento.update( + `${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`, + undefined, + ), ]); - const usage = this.getDeploymentUsage().filter( - (u) => u.safeHostname !== safeHostname, - ); - await this.memento.update(DEPLOYMENT_USAGE_KEY, usage); } /** * Get all known hostnames, ordered by most recently accessed. + * Derives the list from actual session secrets stored. */ - public getKnownSafeHostnames(): string[] { - return this.getDeploymentUsage().map((u) => u.safeHostname); - } - - /** - * Get the full deployment usage list with access timestamps. - */ - private getDeploymentUsage(): DeploymentUsage[] { - const data = this.memento.get(DEPLOYMENT_USAGE_KEY); - if (!data) { - return []; - } - const result = z.array(DeploymentUsageSchema).safeParse(data); - return result.success ? result.data : []; + public async getKnownSafeHostnames(): Promise { + const keys = await this.secrets.keys(); + const sessionHostnames = keys + .filter((k) => k.startsWith(SESSION_KEY_PREFIX)) + .map((k) => k.slice(SESSION_KEY_PREFIX.length)); + + // Sort by access timestamp (most recent first) + const withTimestamps = sessionHostnames.map((hostname) => ({ + hostname, + accessedAt: + this.memento.get(`${DEPLOYMENT_ACCESS_PREFIX}${hostname}`) ?? + "", + })); + + withTimestamps.sort((a, b) => b.accessedAt.localeCompare(a.accessedAt)); + return withTimestamps.map((w) => w.hostname); } /** diff --git a/src/extension.ts b/src/extension.ts index e873eed7..f4214dce 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -11,7 +11,6 @@ import { AuthInterceptor } from "./api/authInterceptor"; import { CoderApi } from "./api/coderApi"; import { Commands } from "./commands"; import { ServiceContainer } from "./core/container"; -import { type SecretsManager } from "./core/secretsManager"; import { DeploymentManager } from "./deployment/deploymentManager"; import { CertificateError } from "./error/certificateError"; import { getErrorDetail, toError } from "./error/errorUtils"; @@ -243,7 +242,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { showTreeViewSearch(ALL_WORKSPACES_TREE_ID), ), vscode.commands.registerCommand("coder.debug.listDeployments", () => - listStoredDeployments(secretsManager), + listStoredDeployments(serviceContainer), ), ); @@ -387,26 +386,36 @@ async function showTreeViewSearch(id: string): Promise { } async function listStoredDeployments( - secretsManager: SecretsManager, + serviceContainer: ServiceContainer, ): Promise { - const hostnames = secretsManager.getKnownSafeHostnames(); - if (hostnames.length === 0) { - vscode.window.showInformationMessage("No deployments stored."); - return; - } + const secretsManager = serviceContainer.getSecretsManager(); + const output = serviceContainer.getLogger(); - const selected = await vscode.window.showQuickPick( - hostnames.map((hostname) => ({ - label: hostname, - description: "Click to forget", - })), - { placeHolder: "Select a deployment to forget" }, - ); + try { + const hostnames = await secretsManager.getKnownSafeHostnames(); + if (hostnames.length === 0) { + vscode.window.showInformationMessage("No deployments stored."); + return; + } + + const selected = await vscode.window.showQuickPick( + hostnames.map((hostname) => ({ + label: hostname, + description: "Click to forget", + })), + { placeHolder: "Select a deployment to forget" }, + ); - if (selected) { - await secretsManager.clearAllAuthData(selected.label); - vscode.window.showInformationMessage( - `Cleared auth data for ${selected.label}`, + if (selected) { + await secretsManager.clearAllAuthData(selected.label); + vscode.window.showInformationMessage( + `Cleared auth data for ${selected.label}`, + ); + } + } catch (error: unknown) { + output.error("Failed to list stored deployments", error); + vscode.window.showErrorMessage( + "Failed to list stored deployments. Storage may be corrupted.", ); } } diff --git a/test/unit/core/secretsManager.test.ts b/test/unit/core/secretsManager.test.ts index 3d1f1e77..5c2b486f 100644 --- a/test/unit/core/secretsManager.test.ts +++ b/test/unit/core/secretsManager.test.ts @@ -75,20 +75,26 @@ describe("SecretsManager", () => { }); it("should track known safe hostnames", async () => { - expect(secretsManager.getKnownSafeHostnames()).toEqual([]); + expect(await secretsManager.getKnownSafeHostnames()).toEqual([]); await secretsManager.setSessionAuth("example.com", { url: "https://example.com", token: "test-token", }); - expect(secretsManager.getKnownSafeHostnames()).toContain("example.com"); + expect(await secretsManager.getKnownSafeHostnames()).toContain( + "example.com", + ); await secretsManager.setSessionAuth("other-com", { url: "https://other.com", token: "other-token", }); - expect(secretsManager.getKnownSafeHostnames()).toContain("example.com"); - expect(secretsManager.getKnownSafeHostnames()).toContain("other-com"); + expect(await secretsManager.getKnownSafeHostnames()).toContain( + "example.com", + ); + expect(await secretsManager.getKnownSafeHostnames()).toContain( + "other-com", + ); }); it("should remove safe hostname on clearAllAuthData", async () => { @@ -96,51 +102,76 @@ describe("SecretsManager", () => { url: "https://example.com", token: "test-token", }); - expect(secretsManager.getKnownSafeHostnames()).toContain("example.com"); + expect(await secretsManager.getKnownSafeHostnames()).toContain( + "example.com", + ); await secretsManager.clearAllAuthData("example.com"); - expect(secretsManager.getKnownSafeHostnames()).not.toContain( + expect(await secretsManager.getKnownSafeHostnames()).not.toContain( "example.com", ); }); + it("should throw when secrets storage is corrupted", async () => { + await secretsManager.setSessionAuth("example.com", { + url: "https://example.com", + token: "test-token", + }); + secretStorage.corruptStorage(); + + await expect(secretsManager.getKnownSafeHostnames()).rejects.toThrow(); + }); + it("should order safe hostnames by most recently accessed", async () => { + vi.useFakeTimers(); + await secretsManager.setSessionAuth("first.com", { url: "https://first.com", token: "token1", }); + vi.advanceTimersByTime(10); await secretsManager.setSessionAuth("second.com", { url: "https://second.com", token: "token2", }); + vi.advanceTimersByTime(10); await secretsManager.setSessionAuth("first.com", { url: "https://first.com", token: "token1-updated", }); - expect(secretsManager.getKnownSafeHostnames()).toEqual([ + expect(await secretsManager.getKnownSafeHostnames()).toEqual([ "first.com", "second.com", ]); + + vi.useRealTimers(); }); it("should prune old deployments when exceeding maxCount", async () => { + // Use fake timers to ensure distinct timestamps for each host + vi.useFakeTimers(); + for (let i = 1; i <= 5; i++) { await secretsManager.setSessionAuth(`host${i}.com`, { url: `https://host${i}.com`, token: `token${i}`, }); + vi.advanceTimersByTime(10); } - await secretsManager.recordDeploymentAccess("new.com", 3); + // With maxCount=3, only the 3 most recently accessed hosts should remain. + await secretsManager.recordDeploymentAccess("host5.com", 3); - expect(secretsManager.getKnownSafeHostnames()).toEqual([ - "new.com", + expect(await secretsManager.getKnownSafeHostnames()).toEqual([ "host5.com", "host4.com", + "host3.com", ]); expect(await secretsManager.getSessionAuth("host1.com")).toBeUndefined(); expect(await secretsManager.getSessionAuth("host2.com")).toBeUndefined(); + + vi.useRealTimers(); }); }); @@ -382,11 +413,6 @@ describe("SecretsManager", () => { ); expect(await secretsManager.getCurrentDeployment()).toBeNull(); }); - - it("returns empty array for invalid deployment usage data", async () => { - await memento.update("coder.deploymentUsage", [{ safeHostname: 123 }]); - expect(secretsManager.getKnownSafeHostnames()).toEqual([]); - }); }); describe("backwards compatibility", () => {