fix: macOS-only keychain + migration; keep Linux/Windows on legacy

This commit is contained in:
kvyb
2025-10-06 19:02:13 +08:00
parent 5847032f82
commit a5bdef041a
7 changed files with 139 additions and 145 deletions
+6 -1
View File
@@ -1,7 +1,12 @@
import { Disposable } from "vscode"
import { Logger } from "@/services/logging/Logger"
import { StorageEventListener } from "./utils/types"
// Lightweight Disposable compatible with vscode.Disposable without importing vscode,
// to keep this module host-agnostic (JetBrains/core runtimes don't have 'vscode').
export interface Disposable {
dispose(): void
}
/**
* An abstract storage class that provides a template for storage operations.
* Subclasses must implement the protected abstract methods to define their storage logic.
+2 -5
View File
@@ -16,11 +16,8 @@ function hasCommand(cmd: string): boolean {
return result.status === 0
}
// Skip on Windows (validated via E2E using the shell), and skip if required tools missing on macOS/Linux
const shouldSkip =
platform === "win32" ||
(platform === "darwin" && !hasCommand("security")) ||
(platform === "linux" && !hasCommand("secret-tool"))
// Only run on macOS for now; skip Windows/Linux
const shouldSkip = platform !== "darwin" || !hasCommand("security")
describe("CredentialStorage", () => {
if (shouldSkip) {
+1 -1
View File
@@ -2,7 +2,7 @@ import type { SecretStorage as VSCodeSecretStorage } from "vscode"
import { Logger } from "@/services/logging/Logger"
import { ClineStorage } from "./ClineStorage"
type SecretStores = VSCodeSecretStorage | ClineStorage
export type SecretStores = VSCodeSecretStorage | ClineStorage
/**
* Wrapper around VSCode Secret Storage or any other storage type for managing secrets.
+25 -46
View File
@@ -5,6 +5,7 @@ import path from "path"
import type { Extension, ExtensionContext } from "vscode"
import { ExtensionKind, ExtensionMode } from "vscode"
import { URI } from "vscode-uri"
import { ClineStorage } from "@/core/storage/ClineStorage"
import { CredentialStorage } from "@/core/storage/credential"
import { FileBasedStorage } from "@/core/storage/file"
import { secretStorage } from "@/core/storage/secrets"
@@ -20,7 +21,7 @@ const SETTINGS_SUBFOLDER = "data"
// Module-level vars used by migration/helpers
let STANDALONE_DEPS_WARNING: string | undefined
let SECRETS_FILE: string
let standaloneBackend: SecretStores | null = null
let standaloneBackend: ClineStorage | null = null
export function initializeContext(clineDir?: string) {
const CLINE_DIR = clineDir || process.env.CLINE_DIR || `${os.homedir()}/.cline`
@@ -94,58 +95,29 @@ export function initializeContext(clineDir?: string) {
}
}
// Select the best standalone secret storage backend (OS keychain when available, else file)
// Select the best standalone secret storage backend
// For now: macOS -> OS keychain; Linux/Windows -> legacy file-based storage
function selectStandaloneSecrets(dataDir: string) {
try {
if (isMacSecurityAvailable()) {
return new CredentialStorage()
}
if (isLinuxSecretToolAvailable()) {
return new CredentialStorage()
} else if (process.platform === "linux") {
STANDALONE_DEPS_WARNING =
"OS keychain tools not found (secret-tool/libsecret). Falling back to file storage. Install: sudo apt-get install -y libsecret-1-0 libsecret-tools dbus gnome-keyring"
}
if (isWindowsCredentialManagerReady()) {
return new CredentialStorage()
} else if (process.platform === "win32") {
STANDALONE_DEPS_WARNING =
"Windows CredentialManager PowerShell module not available. Falling back to file storage. Install in PowerShell: Install-Module -Name CredentialManager -Scope CurrentUser; then restart Cline"
if (process.platform === "darwin") {
if (hasCommand("security")) {
return new CredentialStorage()
} else {
STANDALONE_DEPS_WARNING = "macOS 'security' tool not available; using file-based secrets"
return new FileBasedStorage(path.join(dataDir, "secrets.json"))
}
}
} catch (error) {
log(`Credential backend selection error; falling back to file store: ${String(error)}`)
}
// Default: legacy file-based storage on non-macOS
return new FileBasedStorage(path.join(dataDir, "secrets.json"))
}
function isMacSecurityAvailable(): boolean {
return process.platform === "darwin" && hasCommand("security")
}
function isLinuxSecretToolAvailable(): boolean {
return process.platform === "linux" && hasCommand("secret-tool")
}
function isWindowsCredentialManagerReady(): boolean {
if (process.platform !== "win32") return false
try {
const result = spawnSync("powershell.exe", [
"-NoProfile",
"-ExecutionPolicy",
"Bypass",
"-Command",
"if (Get-Module -ListAvailable -Name CredentialManager) { exit 0 } else { exit 1 }",
])
return result.status === 0
} catch {
return false
}
}
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
}
@@ -154,11 +126,15 @@ function hasCommand(cmd: string): boolean {
async function migrateFileSecretsToOS(filePath: string): Promise<void> {
try {
const fs = await import("fs")
if (!fs.existsSync(filePath)) return
if (!fs.existsSync(filePath)) {
return
}
const raw = fs.readFileSync(filePath, "utf-8")
const data = raw ? (JSON.parse(raw) as Record<string, string>) : {}
const entries = Object.entries(data).filter(([, v]) => typeof v === "string" && v.length > 0)
if (entries.length === 0) return fs.unlinkSync(filePath)
if (entries.length === 0) {
return fs.unlinkSync(filePath)
}
// Parallel pre-check: determine which entries already exist in OS storage
const existingValues = await Promise.all(entries.map(([key]) => secretStorage.get(key)))
@@ -207,7 +183,7 @@ async function migrateFileSecretsToOS(filePath: string): Promise<void> {
export async function runLegacySecretsMigrationIfNeeded(): Promise<void> {
try {
if (standaloneBackend instanceof CredentialStorage) {
if (process.platform === "darwin" && standaloneBackend instanceof CredentialStorage) {
log("Starting legacy secrets migration to OS keychain...")
await migrateFileSecretsToOS(SECRETS_FILE)
log("Legacy secrets migration completed.")
@@ -220,6 +196,9 @@ export async function runLegacySecretsMigrationIfNeeded(): Promise<void> {
}
}
// Test-only export to directly invoke migration logic without starting services
export const __test_migrateFileSecretsToOS = migrateFileSecretsToOS
// Expose any dependency warning to be shown by the host after initialization
export function getStandaloneDepsWarning(): string | undefined {
return STANDALONE_DEPS_WARNING
+15 -75
View File
@@ -4,7 +4,9 @@ import { expect } from "@playwright/test"
import { e2e } from "./utils/helpers"
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
}
@@ -26,103 +28,41 @@ function macDelete(service: string, account: string): boolean {
return res.status === 0
}
function linuxStore(service: string, account: string, value: string): boolean {
const cmd = `printf %s ${JSON.stringify(value)} | secret-tool store --label=${JSON.stringify(service)} service ${JSON.stringify(
service,
)} account ${JSON.stringify(account)}`
const res = spawnSync("sh", ["-c", cmd], { stdio: "ignore" })
return res.status === 0
}
function linuxGet(service: string, account: string): string | undefined {
const res = spawnSync("secret-tool", ["lookup", "service", service, "account", account], { encoding: "utf8" })
return res.status === 0 ? res.stdout.trim() : undefined
}
function linuxDelete(service: string, account: string): boolean {
const res = spawnSync("secret-tool", ["clear", "service", service, "account", account], { stdio: "ignore" })
return res.status === 0
}
function winStore(service: string, account: string, value: string): boolean {
// Requires CredentialManager module
const ps = `New-StoredCredential -Target '${service}:${account}' -Username '${service}' -Password '${value}' -Persist LocalMachine -Type Generic`
const res = spawnSync("powershell.exe", ["-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", ps], { stdio: "ignore" })
return res.status === 0
}
function winGet(service: string, account: string): string | undefined {
const ps = `$c = Get-StoredCredential -Target '${service}:${account}'; if ($c) { Write-Output $c.Password }`
const res = spawnSync("powershell.exe", ["-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", ps], { encoding: "utf8" })
return res.status === 0 ? res.stdout.trim() : undefined
}
function winDelete(service: string, account: string): boolean {
const ps = `Remove-StoredCredential -Target '${service}:${account}' -ErrorAction SilentlyContinue`
const res = spawnSync("powershell.exe", ["-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", ps], { stdio: "ignore" })
return res.status === 0
}
// Linux/Windows helpers removed for now since test is macOS-only
// Extension-host validation of OS keychain commands
e2e("Secrets - OS keychain get/store/delete", async () => {
e2e("Secrets - OS keychain get/store/delete (macOS only)", async () => {
const platform = os.platform()
const service = "cline"
const key = `e2e_secret_${Date.now()}`
const value = "test-secret"
if (platform === "darwin" && !hasCommand("security")) {
if (platform !== "darwin") {
console.warn("Skipping: test is macOS-only for now")
return
}
if (!hasCommand("security")) {
console.warn("Skipping: security CLI not available on macOS runner")
return
}
if (platform === "linux" && !hasCommand("secret-tool")) {
console.warn("Skipping: secret-tool not available on Linux runner")
return
}
if (platform === "win32") {
const check = spawnSync(
"powershell.exe",
[
"-NoProfile",
"-ExecutionPolicy",
"Bypass",
"-Command",
"if (Get-Module -ListAvailable -Name CredentialManager) { exit 0 } else { exit 1 }",
],
{ stdio: "ignore" },
)
if (check.status !== 0) {
console.warn("Skipping: CredentialManager module not available on Windows runner")
return
}
}
// Ensure clean slate
if (platform === "darwin") macDelete(service, key)
if (platform === "linux") linuxDelete(service, key)
if (platform === "win32") winDelete(service, key)
macDelete(service, key)
// Store
const stored =
platform === "darwin"
? macStore(service, key, value)
: platform === "linux"
? linuxStore(service, key, value)
: winStore(service, key, value)
const stored = macStore(service, key, value)
expect(stored).toBeTruthy()
// Get
const fetched =
platform === "darwin" ? macGet(service, key) : platform === "linux" ? linuxGet(service, key) : winGet(service, key)
const fetched = macGet(service, key)
expect(fetched).toBe(value)
// Delete
const deleted =
platform === "darwin"
? macDelete(service, key)
: platform === "linux"
? linuxDelete(service, key)
: winDelete(service, key)
const deleted = macDelete(service, key)
expect(deleted).toBeTruthy()
const after =
platform === "darwin" ? macGet(service, key) : platform === "linux" ? linuxGet(service, key) : winGet(service, key)
const after = macGet(service, key)
expect(after).toBeUndefined()
})
+28 -17
View File
@@ -10,20 +10,16 @@ function hasCommand(cmd: string): boolean {
return result.status === 0
}
test("Standalone migration moves secrets.json to OS keychain and removes file", async () => {
test("Standalone migration moves secrets.json to OS keychain and removes file (macOS only)", async () => {
const platform = os.platform()
if (platform === "win32") {
test.skip(true, "Skip on Windows; covered by shell/E2E elsewhere")
if (platform !== "darwin") {
test.skip(true, "Only macOS migration is supported at this time")
return
}
if (platform === "darwin" && !hasCommand("security")) {
if (!hasCommand("security")) {
test.skip(true, "security CLI not available on macOS runner")
return
}
if (platform === "linux" && !hasCommand("secret-tool")) {
test.skip(true, "secret-tool not available on Linux runner")
return
}
// Prepare isolated CLINE_DIR with legacy secrets.json
const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), "cline-standalone-mig-"))
@@ -35,11 +31,7 @@ test("Standalone migration moves secrets.json to OS keychain and removes file",
// Ensure no preexisting key in OS keychain
const service = "Cline: openRouterApiKey"
const account = "cline_openRouterApiKey"
if (platform === "darwin") {
spawnSync("security", ["delete-generic-password", "-s", service, "-a", account], { stdio: "ignore" })
} else if (platform === "linux") {
spawnSync("secret-tool", ["clear", "service", service, "account", account], { stdio: "ignore" })
}
spawnSync("security", ["delete-generic-password", "-s", service, "-a", account], { stdio: "ignore" })
// Start the standalone core service via helper script
const procEnv = {
@@ -93,10 +85,9 @@ test("Standalone migration moves secrets.json to OS keychain and removes file",
let present = false
const checkStart = Date.now()
while (Date.now() - checkStart < 5000) {
const status =
platform === "darwin"
? spawnSync("security", ["find-generic-password", "-s", service, "-a", account, "-w"], { stdio: "ignore" }).status
: spawnSync("secret-tool", ["lookup", "service", service, "account", account], { stdio: "ignore" }).status
const status = spawnSync("security", ["find-generic-password", "-s", service, "-a", account, "-w"], {
stdio: "ignore",
}).status
if (status === 0) {
present = true
break
@@ -104,6 +95,26 @@ test("Standalone migration moves secrets.json to OS keychain and removes file",
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 },
)
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 () => {
@@ -0,0 +1,62 @@
import * as fs from "node:fs"
import * as os from "node:os"
import * as path from "node:path"
import { expect } from "chai"
import sinon from "sinon"
import { secretStorage } from "@/core/storage/secrets"
import { __test_migrateFileSecretsToOS as migrate } from "@/standalone/vscode-context"
describe("Standalone secrets migration (unit)", () => {
const platform = os.platform()
if (platform !== "darwin") {
it("skipped on non-macOS", () => {
expect(true).to.equal(true)
})
return
}
let tmpDir: string
let secretsPath: string
let storeStub: sinon.SinonStub
let deleteStub: sinon.SinonStub
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cline-mig-unit-"))
secretsPath = path.join(tmpDir, "secrets.json")
fs.writeFileSync(secretsPath, JSON.stringify({ a: "1", b: "2" }, null, 2))
sinon.stub(secretStorage as any, "get").resolves(undefined)
storeStub = sinon.stub(secretStorage as any, "store").resolves()
deleteStub = sinon.stub(secretStorage as any, "delete").resolves()
})
afterEach(() => {
try {
fs.rmSync(tmpDir, { recursive: true, force: true })
} catch {}
sinon.restore()
})
it("migrates all entries and removes file", async () => {
await migrate(secretsPath)
expect(fs.existsSync(secretsPath)).to.equal(false)
expect(storeStub.callCount).to.equal(2)
expect(deleteStub.called).to.equal(false)
})
it("rolls back on failure and keeps file", async () => {
// Fail on second store
let count = 0
storeStub.callsFake(async () => {
count++
if (count === 2) {
throw new Error("simulated failure")
}
})
await migrate(secretsPath)
// rollback called for first key
expect(deleteStub.callCount).to.equal(1)
// legacy file preserved
expect(fs.existsSync(secretsPath)).to.equal(true)
})
})