fix: testing works with keychains, migration verification, and CRUD operations. Fixes security CLI argument ordering and credential.ts error handling.

This commit is contained in:
kvyb
2025-10-07 00:22:43 +08:00
parent 052ff38e16
commit f3e0337f73
7 changed files with 215 additions and 103 deletions
+13
View File
@@ -197,6 +197,19 @@ async function main() {
} else {
await extensionCtx.rebuild()
await extensionCtx.dispose()
// Also build the migration test utility for E2E tests when building standalone
if (standalone) {
const migrationConfig = {
...baseConfig,
entryPoints: ["src/test/e2e/utils/migrate-secrets.ts"],
outfile: `${destDir}/migrate-secrets.js`,
external: ["vscode", "@grpc/reflection", "grpc-health-check", "better-sqlite3"],
}
const migrationCtx = await esbuild.context(migrationConfig)
await migrationCtx.rebuild()
await migrationCtx.dispose()
}
}
}
+10 -3
View File
@@ -79,7 +79,8 @@ async function main(): Promise<void> {
}
const extensionsDir = path.join(distDir, "vsce-extension")
const userDataDir = mkdtempSync(path.join(os.tmpdir(), "vsce"))
// Respect incoming CLINE_DIR from parent (e.g., E2E tests that seed legacy secrets)
const userDataDir = process.env.CLINE_DIR ? process.env.CLINE_DIR : mkdtempSync(path.join(os.tmpdir(), "vsce"))
const clineTestWorkspace = mkdtempSync(path.join(os.tmpdir(), "cline-test-workspace-"))
console.log("Starting HostBridge test server...")
@@ -134,16 +135,22 @@ async function main(): Promise<void> {
CLINE_DIR: userDataDir,
INSTALL_DIR: extensionsDir,
},
stdio: "inherit",
stdio: "pipe",
})
childProcesses.push(coreService)
// Proxy core service output so callers that pipe this script can observe logs
coreService.stdout?.on("data", (chunk) => process.stdout.write(chunk))
coreService.stderr?.on("data", (chunk) => process.stderr.write(chunk))
const shutdown = async () => {
console.log("\nShutting down services...")
while (childProcesses.length > 0) {
const child = childProcesses.pop()
if (child && !child.killed) child.kill("SIGINT")
if (child && !child.killed) {
child.kill("SIGINT")
}
}
await ClineApiServerMock.stopGlobalServer()
@@ -4,7 +4,7 @@ import { expect } from "chai"
import * as sinon from "sinon"
import { ErrorService } from "@/services/error"
import { Logger } from "@/services/logging/Logger"
import { CredentialStorage } from "./credential"
import { CredentialStorage } from "../credential"
const platform = os.platform()
+37 -12
View File
@@ -55,6 +55,11 @@ export class CredentialStorage extends ClineStorage {
const result = await this.exec(this.commands.get({ service, account, target }))
return result || undefined
} catch (error) {
// Return undefined if the key doesn't exist (expected behavior)
// "The specified item could not be found" is not an error, just means key doesn't exist
if (error instanceof Error && error.message.includes("could not be found")) {
return undefined
}
throw error
}
}
@@ -111,18 +116,38 @@ export class CredentialStorage extends ClineStorage {
const PLATFORM_COMMANDS: PlatformCommands = {
[PLATFORM_OS.MacOS]: {
get: ({ service, account }) => ({
command: "security",
args: ["find-generic-password", "-s", service, "-a", account, "-w"],
}),
store: ({ service, account, value }) => ({
command: "security",
args: ["add-generic-password", "-s", service, "-a", account, "-w", value],
}),
delete: ({ service, account }) => ({
command: "security",
args: ["delete-generic-password", "-s", service, "-a", account],
}),
get: ({ service, account }) => {
const keychain = process.env.CLINE_KEYCHAIN
const args = ["find-generic-password", "-s", service, "-a", account, "-w"]
// Note: find-generic-password doesn't support -k flag
// If using a custom keychain, specify it at the end as a positional argument
if (keychain && keychain.length > 0) {
args.push(keychain)
}
return { command: "security", args }
},
store: ({ service, account, value }) => {
const keychain = process.env.CLINE_KEYCHAIN
const args = ["add-generic-password", "-s", service, "-a", account]
// For test keychains, allow all apps to access (makes it manageable in Keychain Access)
if (keychain && keychain.length > 0) {
args.push("-A")
}
args.push("-w", value)
// Keychain must be specified as positional argument at the end
if (keychain && keychain.length > 0) {
args.push(keychain)
}
return { command: "security", args }
},
delete: ({ service, account }) => {
const keychain = process.env.CLINE_KEYCHAIN
const args = ["delete-generic-password", "-s", service, "-a", account]
if (keychain && keychain.length > 0) {
args.push("-k", keychain)
}
return { command: "security", args }
},
},
[PLATFORM_OS.Linux]: {
get: ({ service, account }) => ({
+12 -2
View File
@@ -1,5 +1,5 @@
import { spawnSync } from "node:child_process"
import { mkdirSync } from "fs"
import { existsSync, mkdirSync } from "fs"
import os from "os"
import path from "path"
import type { Extension, ExtensionContext } from "vscode"
@@ -41,12 +41,22 @@ export function initializeContext(clineDir?: string) {
const EXTENSION_DIR = path.join(INSTALL_DIR, "extension")
const EXTENSION_MODE = process.env.IS_DEV === "true" ? ExtensionMode.Development : ExtensionMode.Production
// In minimal contexts (e.g., migration tests), the packaged extension assets may not exist.
// Read package.json best-effort to avoid crashing during migrate-only runs.
let extensionPackageJson: any
try {
const pkgPath = path.join(EXTENSION_DIR, "package.json")
extensionPackageJson = existsSync(pkgPath) ? readJson(pkgPath) : { name: "cline-standalone", version: "0.0.0" }
} catch {
extensionPackageJson = { name: "cline-standalone", version: "0.0.0" }
}
const extension: Extension<void> = {
id: ExtensionRegistryInfo.id,
isActive: true,
extensionPath: EXTENSION_DIR,
extensionUri: URI.file(EXTENSION_DIR),
packageJSON: readJson(path.join(EXTENSION_DIR, "package.json")),
packageJSON: extensionPackageJson,
exports: undefined, // There are no API exports in the standalone version.
activate: async () => {},
extensionKind: ExtensionKind.UI,
+111 -85
View File
@@ -5,11 +5,15 @@ import * as path from "node:path"
import { expect, test } from "@playwright/test"
function hasCommand(cmd: string): boolean {
if (process.platform === "win32") return true
if (process.platform === "win32") {
return true
}
const result = spawnSync("sh", ["-c", `command -v ${cmd}`], { stdio: "ignore" })
return result.status === 0
}
test.setTimeout(90000)
test("Standalone migration moves secrets.json to OS keychain and removes file (macOS only)", async () => {
const platform = os.platform()
if (platform !== "darwin") {
@@ -28,93 +32,111 @@ test("Standalone migration moves secrets.json to OS keychain and removes file (m
const secretsPath = path.join(dataDir, "secrets.json")
fs.writeFileSync(secretsPath, JSON.stringify({ openRouterApiKey: "migrate-me" }, null, 2))
// Ensure no preexisting key in OS keychain
const service = "Cline: openRouterApiKey"
const account = "cline_openRouterApiKey"
spawnSync("security", ["delete-generic-password", "-s", service, "-a", account], { stdio: "ignore" })
// Start the standalone core service via helper script
const procEnv = {
...process.env,
CLINE_DIR: userDataDir,
E2E_TEST: "true",
CLINE_ENVIRONMENT: "local",
}
const server: ChildProcess = spawn(
process.platform === "win32" ? "npx.cmd" : "npx",
["tsx", "scripts/test-standalone-core-api-server.ts"],
{ stdio: "pipe", env: procEnv },
)
// Capture logs
let output = ""
server.stdout?.on("data", (d) => (output += String(d)))
server.stderr?.on("data", (d) => (output += String(d)))
// Helper to wait for a regex in output up to timeoutMs
async function waitFor(pattern: RegExp, timeoutMs: number): Promise<boolean> {
const start = Date.now()
while (Date.now() - start < timeoutMs) {
if (pattern.test(output)) return true
await new Promise((r) => setTimeout(r, 200))
}
return false
}
// Wait for migration complete log (deterministic), up to 45s
const successSeen = await waitFor(
/Secrets migration: (migrated .* entries; removed secrets\.json|all entries already present; removed secrets\.json)/i,
45000,
)
// If an abort log appeared, surface it
const abortSeen =
/Secrets migration aborted and rolled back/i.test(output) || /Migration from secrets\.json failed/i.test(output)
// Stop server
// Ephemeral, unlocked test keychain to avoid GUI prompts
const keychainPath = path.join(os.tmpdir(), `cline-tests-${Date.now()}-${Math.random().toString(36).slice(2)}.keychain-db`)
const keychainPwd = "cline-test-pass"
try {
server.kill("SIGINT")
} catch {}
spawnSync("security", ["create-keychain", "-p", keychainPwd, keychainPath], { stdio: "ignore" })
spawnSync("security", ["set-keychain-settings", "-lut", "3600", keychainPath], { stdio: "ignore" })
spawnSync("security", ["unlock-keychain", "-p", keychainPwd, keychainPath], { stdio: "ignore" })
expect(abortSeen).toBeFalsy()
expect(successSeen).toBeTruthy()
// File should be removed
expect(fs.existsSync(secretsPath)).toBeFalsy()
// Ensure no preexisting key in test keychain
const service = "Cline: openRouterApiKey"
const account = "cline_openRouterApiKey"
spawnSync("security", ["delete-generic-password", "-s", service, "-a", account, "-k", keychainPath], { stdio: "ignore" })
// Quick keychain presence check (5s max)
let present = false
const checkStart = Date.now()
while (Date.now() - checkStart < 5000) {
const status = spawnSync("security", ["find-generic-password", "-s", service, "-a", account, "-w"], {
stdio: "ignore",
}).status
if (status === 0) {
present = true
break
// Run the migration script (compiled from standalone build)
const procEnv = { ...process.env, CLINE_DIR: userDataDir, CLINE_KEYCHAIN: keychainPath }
// Verify migration script exists
const migrationScript = path.join(process.cwd(), "dist-standalone", "migrate-secrets.js")
if (!fs.existsSync(migrationScript)) {
throw new Error("Migration script not found. Run: npm run compile-standalone")
}
await new Promise((r) => setTimeout(r, 250))
const migrator: ChildProcess = spawn("node", [migrationScript], { stdio: "pipe", env: procEnv })
let output = ""
migrator.stdout?.on("data", (d) => {
output += String(d)
})
migrator.stderr?.on("data", (d) => {
output += String(d)
})
// Wait for migration to complete by polling for file removal (source of truth)
const end = Date.now() + 60000
let successSeen = false
let abortSeen = false
while (Date.now() < end) {
// If we saw an abort, surface immediately
abortSeen = /Secrets migration aborted and rolled back|Migration from secrets\.json failed/i.test(output)
if (abortSeen) {
break
}
if (!fs.existsSync(secretsPath)) {
successSeen = true
break
}
await new Promise((r) => setTimeout(r, 250))
}
try {
migrator.kill("SIGINT")
} catch {
// Ignore if already exited
}
if (abortSeen || !successSeen) {
console.log("Migration output:", output)
}
expect(abortSeen).toBeFalsy()
expect(successSeen).toBeTruthy()
// File should be removed
expect(fs.existsSync(secretsPath)).toBeFalsy()
// Quick keychain presence check (5s max)
let present = false
const checkStart = Date.now()
while (Date.now() - checkStart < 5000) {
// Keychain path must be positional argument at the end, not with -k flag
const status = spawnSync("security", ["find-generic-password", "-s", service, "-a", account, "-w", keychainPath], {
stdio: "ignore",
}).status
if (status === 0) {
present = true
break
}
await new Promise((r) => setTimeout(r, 250))
}
expect(present).toBeTruthy()
// Second run: ensure no re-migration and legacy file is not recreated
const server2: ChildProcess = spawn(
process.platform === "win32" ? "npx.cmd" : "npx",
["tsx", "scripts/test-standalone-core-api-server.ts"],
{ stdio: "pipe", env: procEnv },
)
// Capture output for debugging if needed
server2.stdout?.on("data", () => {
// Output captured but not used - server just needs to run
})
server2.stderr?.on("data", () => {
// Output captured but not used - server just needs to run
})
// Give it a short window to initialize
await new Promise((r) => setTimeout(r, 4000))
try {
server2.kill("SIGINT")
} catch {}
// secrets.json should not have been recreated
expect(fs.existsSync(secretsPath)).toBeFalsy()
} finally {
// Clean up test keychain
spawnSync("security", ["delete-keychain", keychainPath], { stdio: "ignore" })
}
expect(present).toBeTruthy()
// Second run: ensure no re-migration and legacy file is not recreated
const server2: ChildProcess = spawn(
process.platform === "win32" ? "npx.cmd" : "npx",
["tsx", "scripts/test-standalone-core-api-server.ts"],
{ stdio: "pipe", env: procEnv },
)
let outputSecond = ""
server2.stdout?.on("data", (d) => (outputSecond += String(d)))
server2.stderr?.on("data", (d) => (outputSecond += String(d)))
// Give it a short window to initialize
await new Promise((r) => setTimeout(r, 4000))
try {
server2.kill("SIGINT")
} catch {}
// secrets.json should not have been recreated
expect(fs.existsSync(secretsPath)).toBeFalsy()
})
test("Standalone deps warning emitted when deps missing (best-effort)", async () => {
@@ -136,8 +158,12 @@ test("Standalone deps warning emitted when deps missing (best-effort)", async ()
)
let output2 = ""
server.stdout?.on("data", (d) => (output2 += String(d)))
server.stderr?.on("data", (d) => (output2 += String(d)))
server.stdout?.on("data", (d) => {
output2 += String(d)
})
server.stderr?.on("data", (d) => {
output2 += String(d)
})
await new Promise((r) => setTimeout(r, 6000))
try {
+31
View File
@@ -0,0 +1,31 @@
#!/usr/bin/env node
/**
* Minimal migration script for E2E testing.
* This runs only the migration logic without starting the full server.
*/
import { initializeContext, runLegacySecretsMigrationIfNeeded } from "../../../standalone/vscode-context"
async function main() {
const clineDir = process.env.CLINE_DIR
if (!clineDir) {
console.error("CLINE_DIR environment variable not set")
process.exit(1)
}
try {
// Initialize context (sets up storage backends)
initializeContext(clineDir)
// Run migration
await runLegacySecretsMigrationIfNeeded()
console.log("MIGRATION_DONE")
process.exit(0)
} catch (error) {
console.error("MIGRATION_FAILED", error)
process.exit(1)
}
}
main()