From 11b94a802d4a0d08cee54d1c1f7eb262eb7bc908 Mon Sep 17 00:00:00 2001 From: kvyb Date: Tue, 23 Sep 2025 08:40:51 +0800 Subject: [PATCH] fix: make legacy secrets migration synchronous with logging; keep graceful fallback --- src/standalone/cline-core.ts | 11 +++++- src/standalone/vscode-context.ts | 20 ++++++++--- src/test/e2e/secrets.test.ts | 57 ++++++++++++++++++++++++++++++++ src/test/e2e/utils/helpers.ts | 26 +++++++++++++++ 4 files changed, 108 insertions(+), 6 deletions(-) diff --git a/src/standalone/cline-core.ts b/src/standalone/cline-core.ts index 3ae2a4789b..a51142ebf9 100644 --- a/src/standalone/cline-core.ts +++ b/src/standalone/cline-core.ts @@ -14,7 +14,13 @@ import { ShowMessageType } from "@/shared/proto/host/window" import { waitForHostBridgeReady } from "./hostbridge-client" import { startProtobusService } from "./protobus-service" import { log } from "./utils" -import { DATA_DIR, EXTENSION_DIR, extensionContext, getStandaloneDepsWarning } from "./vscode-context" +import { + DATA_DIR, + EXTENSION_DIR, + extensionContext, + getStandaloneDepsWarning, + runLegacySecretsMigrationIfNeeded, +} from "./vscode-context" async function main() { log("\n\n\nStarting cline-core service...\n\n\n") @@ -27,6 +33,9 @@ async function main() { // Set up global error handlers to prevent process crashes setupGlobalErrorHandlers() + // Ensure legacy secrets are migrated to OS keychain before any reads during initialization + await runLegacySecretsMigrationIfNeeded() + const webviewProvider = await initialize(extensionContext) // Enable the localhost HTTP server that handles auth redirects. diff --git a/src/standalone/vscode-context.ts b/src/standalone/vscode-context.ts index bf86973cfb..06e78daeb9 100644 --- a/src/standalone/vscode-context.ts +++ b/src/standalone/vscode-context.ts @@ -32,11 +32,6 @@ let STANDALONE_DEPS_WARNING: string | undefined const standaloneBackend = selectStandaloneSecrets(DATA_DIR) secretStorage.init(standaloneBackend) -// One-time migration: if using OS credentials and secrets.json exists, migrate entries -if (standaloneBackend instanceof CredentialStorage) { - void migrateFileSecretsToOS(SECRETS_FILE) -} - export const EXTENSION_DIR = path.join(INSTALL_DIR, "extension") const EXTENSION_MODE = process.env.IS_DEV === "true" ? ExtensionMode.Development : ExtensionMode.Production @@ -194,6 +189,21 @@ async function migrateFileSecretsToOS(filePath: string): Promise { } } +export async function runLegacySecretsMigrationIfNeeded(): Promise { + try { + if (standaloneBackend instanceof CredentialStorage) { + log("Starting legacy secrets migration to OS keychain...") + await migrateFileSecretsToOS(SECRETS_FILE) + log("Legacy secrets migration completed.") + } else { + // Graceful fallback to file-based storage; no migration needed + log("OS keychain unavailable; using file-based secrets. Skipping migration.") + } + } catch (error) { + log(`Legacy secrets migration error: ${String(error)}`) + } +} + console.log("Finished loading vscode context...") export { extensionContext } diff --git a/src/test/e2e/secrets.test.ts b/src/test/e2e/secrets.test.ts index 2e8a434bb9..be2ea16ac3 100644 --- a/src/test/e2e/secrets.test.ts +++ b/src/test/e2e/secrets.test.ts @@ -1,5 +1,7 @@ import { spawnSync } from "node:child_process" +import * as fs from "node:fs" import os from "node:os" +import * as path from "node:path" import { expect, test } from "@playwright/test" import { e2e } from "./utils/helpers" @@ -124,6 +126,61 @@ e2e("Secrets - OS keychain get/store/delete", async () => { expect(after).toBeUndefined() }) +e2e("Secrets migration - moves legacy secrets.json to OS keychain and removes file", async () => { + const platform = os.platform() + if (platform !== "darwin" && platform !== "linux") { + test.skip(true, "Migration test only runs on macOS/Linux where OS keychain is available in CI") + return + } + if (platform === "darwin" && !hasCommand("security")) { + test.skip(true, "security CLI not available") + return + } + if (platform === "linux" && !hasCommand("secret-tool")) { + test.skip(true, "secret-tool not available") + return + } + + // Prepare env for helpers to seed secrets.json before app launch + process.env.E2E_MIGRATION_SECRETS_JSON = JSON.stringify({ openRouterApiKey: "migrate-me" }) + + // Launch VS Code to trigger extension startup and migration + const { E2ETestHelper } = await import("./utils/helpers") + const executablePath = await (await import("@vscode/test-electron")).downloadAndUnzipVSCode("stable", undefined) + const { _electron } = await import("playwright") + const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), "vsce-mig")) + const app = await _electron.launch({ + executablePath, + env: { ...process.env, CLINE_DIR: userDataDir, E2E_TEST: "true" }, + args: [ + "--disable-extensions", + "--skip-welcome", + "--skip-release-notes", + `--user-data-dir=${userDataDir}`, + `--install-extension=${path.join(E2ETestHelper.CODEBASE_ROOT_DIR, "dist", "e2e.vsix")}`, + `--extensionDevelopmentPath=${E2ETestHelper.CODEBASE_ROOT_DIR}`, + path.join(E2ETestHelper.E2E_TESTS_DIR, "fixtures", "workspace"), + ], + }) + await E2ETestHelper.waitUntil(() => app.windows().length > 0) + + // Verify key present in OS keychain + const service = "Cline: openRouterApiKey" + const account = "cline_openRouterApiKey" + const present = + platform === "darwin" + ? spawnSync("security", ["find-generic-password", "-s", service, "-a", account, "-w"], { stdio: "ignore" }).status === + 0 + : spawnSync("secret-tool", ["lookup", "service", service, "account", account], { stdio: "ignore" }).status === 0 + expect(present).toBeTruthy() + + // Verify legacy file removed + const secretsPath = path.join(userDataDir, "data", "secrets.json") + expect(fs.existsSync(secretsPath)).toBeFalsy() + + await app.close() +}) + test.describe("Standalone deps warning", () => { e2e("Toast presence matches EXPECT_DEPS env", async ({ page }) => { const expectDeps = process.env.EXPECT_DEPS === "true" diff --git a/src/test/e2e/utils/helpers.ts b/src/test/e2e/utils/helpers.ts index e855a89158..0c3b7d72d3 100644 --- a/src/test/e2e/utils/helpers.ts +++ b/src/test/e2e/utils/helpers.ts @@ -240,6 +240,30 @@ export const e2e = test const executablePath = await downloadAndUnzipVSCode(channel, undefined, new SilentReporter()) await use(async (workspacePath: string) => { + // Prepare CLINE_DIR in the VS Code environment to isolate per-test data + // Optionally pre-create legacy secrets.json for migration tests + try { + const fs = await import("node:fs") + const pathMod = await import("node:path") + const clineDir = userDataDir // Use user data dir as CLINE_DIR for isolation + // Pre-create secrets.json if requested for migration test titles + const wantsMigration = + (process.env.E2E_MIGRATION_SECRETS_JSON || "").length > 0 && + testInfo.title.toLowerCase().includes("migration") + if (wantsMigration) { + const dataDir = pathMod.join(clineDir, "data") + fs.mkdirSync(dataDir, { recursive: true }) + const secretsPath = pathMod.join(dataDir, "secrets.json") + fs.writeFileSync( + secretsPath, + JSON.stringify(JSON.parse(process.env.E2E_MIGRATION_SECRETS_JSON as string), null, 2), + ) + } + } catch {} + + // Expose the chosen CLINE_DIR to the test process for assertions + process.env.LAST_E2E_CLINE_DIR = userDataDir + const app = await _electron.launch({ executablePath, env: { @@ -247,6 +271,8 @@ export const e2e = test TEMP_PROFILE: "true", E2E_TEST: "true", CLINE_ENVIRONMENT: "local", + // Ensure extension uses test-specific data directory + CLINE_DIR: userDataDir, GRPC_RECORDER_FILE_NAME: E2ETestHelper.generateTestFileName(testInfo.title, testInfo.project.name), // GRPC_RECORDER_ENABLED: "true", // GRPC_RECORDER_TESTS_FILTERS_ENABLED: "true"