Skip to content

Commit 3845f34

Browse files
roomoteroomote
committed
fix: implement profile-aware secret storage for VSCode profile isolation (#5442)
- Add profile-specific secret key generation using machineId, appName, and uriScheme - Implement automatic migration from legacy global secrets to profile-specific storage - Ensure Qdrant API keys and other secrets are isolated between VSCode profiles (local, WSL, etc.) - Add comprehensive tests for profile isolation and migration functionality - Maintain backward compatibility with existing secret storage Fixes #5442
1 parent ad201cc commit 3845f34

File tree

2 files changed

+226
-17
lines changed

2 files changed

+226
-17
lines changed

‎src/core/config/ContextProxy.ts‎

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as vscode from "vscode"
22
import { ZodError } from "zod"
3+
import * as crypto from "crypto"
34

45
import {
56
PROVIDER_SETTINGS_KEYS,
@@ -39,12 +40,39 @@ export class ContextProxy {
3940
private stateCache: GlobalState
4041
private secretCache: SecretState
4142
private _isInitialized = false
43+
private _profileId: string
4244

4345
constructor(context: vscode.ExtensionContext) {
4446
this.originalContext = context
4547
this.stateCache = {}
4648
this.secretCache = {}
4749
this._isInitialized = false
50+
this._profileId = this.generateProfileId()
51+
}
52+
53+
/**
54+
* Generates a unique profile identifier based on VSCode's environment
55+
* This ensures secrets are isolated per VSCode profile (local, WSL, etc.)
56+
*/
57+
private generateProfileId(): string {
58+
// Use a combination of machineId and environment-specific identifiers
59+
const machineId = vscode.env.machineId
60+
const appName = vscode.env.appName
61+
const uriScheme = vscode.env.uriScheme
62+
63+
// Create a hash of these identifiers to create a unique profile ID
64+
const profileData = `${machineId}-${appName}-${uriScheme}`
65+
const hash = crypto.createHash("sha256").update(profileData).digest("hex")
66+
67+
// Use first 16 characters for a shorter but still unique identifier
68+
return hash.substring(0, 16)
69+
}
70+
71+
/**
72+
* Creates a profile-specific secret key to ensure secrets are isolated per VSCode profile
73+
*/
74+
private getProfileSpecificSecretKey(key: SecretStateKey): string {
75+
return `${this._profileId}:${key}`
4876
}
4977

5078
public get isInitialized() {
@@ -63,7 +91,24 @@ export class ContextProxy {
6391

6492
const promises = SECRET_STATE_KEYS.map(async (key) => {
6593
try {
66-
this.secretCache[key] = await this.originalContext.secrets.get(key)
94+
// Use profile-specific key for secrets to ensure profile isolation
95+
const profileSpecificKey = this.getProfileSpecificSecretKey(key)
96+
let secretValue = await this.originalContext.secrets.get(profileSpecificKey)
97+
98+
// Migration: If profile-specific secret doesn't exist, check for legacy global secret
99+
if (!secretValue) {
100+
const legacyValue = await this.originalContext.secrets.get(key)
101+
if (legacyValue) {
102+
// Migrate legacy secret to profile-specific storage
103+
await this.originalContext.secrets.store(profileSpecificKey, legacyValue)
104+
// Clean up legacy secret to prevent cross-profile leakage
105+
await this.originalContext.secrets.delete(key)
106+
secretValue = legacyValue
107+
logger.info(`Migrated secret ${key} to profile-specific storage`)
108+
}
109+
}
110+
111+
this.secretCache[key] = secretValue
67112
} catch (error) {
68113
logger.error(`Error loading secret ${key}: ${error instanceof Error ? error.message : String(error)}`)
69114
}
@@ -141,10 +186,13 @@ export class ContextProxy {
141186
// Update cache.
142187
this.secretCache[key] = value
143188

144-
// Write directly to context.
189+
// Use profile-specific key for secrets to ensure profile isolation
190+
const profileSpecificKey = this.getProfileSpecificSecretKey(key)
191+
192+
// Write directly to context with profile-specific key.
145193
return value === undefined
146-
? this.originalContext.secrets.delete(key)
147-
: this.originalContext.secrets.store(key, value)
194+
? this.originalContext.secrets.delete(profileSpecificKey)
195+
: this.originalContext.secrets.store(profileSpecificKey, value)
148196
}
149197

150198
/**
@@ -154,7 +202,24 @@ export class ContextProxy {
154202
async refreshSecrets(): Promise<void> {
155203
const promises = SECRET_STATE_KEYS.map(async (key) => {
156204
try {
157-
this.secretCache[key] = await this.originalContext.secrets.get(key)
205+
// Use profile-specific key for secrets to ensure profile isolation
206+
const profileSpecificKey = this.getProfileSpecificSecretKey(key)
207+
let secretValue = await this.originalContext.secrets.get(profileSpecificKey)
208+
209+
// Migration: If profile-specific secret doesn't exist, check for legacy global secret
210+
if (!secretValue) {
211+
const legacyValue = await this.originalContext.secrets.get(key)
212+
if (legacyValue) {
213+
// Migrate legacy secret to profile-specific storage
214+
await this.originalContext.secrets.store(profileSpecificKey, legacyValue)
215+
// Clean up legacy secret to prevent cross-profile leakage
216+
await this.originalContext.secrets.delete(key)
217+
secretValue = legacyValue
218+
logger.info(`Migrated secret ${key} to profile-specific storage during refresh`)
219+
}
220+
}
221+
222+
this.secretCache[key] = secretValue
158223
} catch (error) {
159224
logger.error(
160225
`Error refreshing secret ${key}: ${error instanceof Error ? error.message : String(error)}`,
@@ -284,7 +349,11 @@ export class ContextProxy {
284349

285350
await Promise.all([
286351
...GLOBAL_STATE_KEYS.map((key) => this.originalContext.globalState.update(key, undefined)),
287-
...SECRET_STATE_KEYS.map((key) => this.originalContext.secrets.delete(key)),
352+
...SECRET_STATE_KEYS.map((key) => {
353+
// Use profile-specific key for secrets to ensure profile isolation
354+
const profileSpecificKey = this.getProfileSpecificSecretKey(key)
355+
return this.originalContext.secrets.delete(profileSpecificKey)
356+
}),
288357
])
289358

290359
await this.initialize()

‎src/core/config/__tests__/ContextProxy.spec.ts‎

Lines changed: 151 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ vi.mock("vscode", () => ({
1515
Production: 2,
1616
Test: 3,
1717
},
18+
env: {
19+
machineId: "test-machine-id",
20+
appName: "Visual Studio Code",
21+
uriScheme: "vscode",
22+
},
1823
}))
1924

2025
describe("ContextProxy", () => {
@@ -76,10 +81,11 @@ describe("ContextProxy", () => {
7681
}
7782
})
7883

79-
it("should initialize secret cache with all secret keys", () => {
84+
it("should initialize secret cache with all secret keys using profile-specific keys", () => {
8085
expect(mockSecrets.get).toHaveBeenCalledTimes(SECRET_STATE_KEYS.length)
8186
for (const key of SECRET_STATE_KEYS) {
82-
expect(mockSecrets.get).toHaveBeenCalledWith(key)
87+
// Should use profile-specific key format: profileId:originalKey
88+
expect(mockSecrets.get).toHaveBeenCalledWith(expect.stringMatching(new RegExp(`^[a-f0-9]{16}:${key}$`)))
8389
}
8490
})
8591
})
@@ -191,22 +197,22 @@ describe("ContextProxy", () => {
191197
})
192198

193199
describe("storeSecret", () => {
194-
it("should store secret directly in original context", async () => {
200+
it("should store secret directly in original context with profile-specific key", async () => {
195201
await proxy.storeSecret("apiKey", "new-secret")
196202

197-
// Should have called original context
198-
expect(mockSecrets.store).toHaveBeenCalledWith("apiKey", "new-secret")
203+
// Should have called original context with profile-specific key
204+
expect(mockSecrets.store).toHaveBeenCalledWith(expect.stringMatching(/^[a-f0-9]{16}:apiKey$/), "new-secret")
199205

200206
// Should have stored the value in cache
201207
const storedValue = await proxy.getSecret("apiKey")
202208
expect(storedValue).toBe("new-secret")
203209
})
204210

205-
it("should handle undefined value for secret deletion", async () => {
211+
it("should handle undefined value for secret deletion with profile-specific key", async () => {
206212
await proxy.storeSecret("apiKey", undefined)
207213

208-
// Should have called delete on original context
209-
expect(mockSecrets.delete).toHaveBeenCalledWith("apiKey")
214+
// Should have called delete on original context with profile-specific key
215+
expect(mockSecrets.delete).toHaveBeenCalledWith(expect.stringMatching(/^[a-f0-9]{16}:apiKey$/))
210216

211217
// Should have stored undefined in cache
212218
const storedValue = await proxy.getSecret("apiKey")
@@ -391,17 +397,19 @@ describe("ContextProxy", () => {
391397
expect(mockGlobalState.update).toHaveBeenCalledTimes(expectedUpdateCalls)
392398
})
393399

394-
it("should delete all secrets", async () => {
400+
it("should delete all secrets using profile-specific keys", async () => {
395401
// Setup initial secrets
396402
await proxy.storeSecret("apiKey", "test-api-key")
397403
await proxy.storeSecret("openAiApiKey", "test-openai-key")
398404

399405
// Reset all state
400406
await proxy.resetAllState()
401407

402-
// Should have called delete for each key
408+
// Should have called delete for each key with profile-specific format
403409
for (const key of SECRET_STATE_KEYS) {
404-
expect(mockSecrets.delete).toHaveBeenCalledWith(key)
410+
expect(mockSecrets.delete).toHaveBeenCalledWith(
411+
expect.stringMatching(new RegExp(`^[a-f0-9]{16}:${key}$`)),
412+
)
405413
}
406414

407415
// Total calls should equal the number of secret keys
@@ -419,4 +427,136 @@ describe("ContextProxy", () => {
419427
expect(initializeSpy).toHaveBeenCalledTimes(1)
420428
})
421429
})
430+
431+
describe("profile-aware secret storage", () => {
432+
it("should generate consistent profile IDs", () => {
433+
// Create multiple instances and verify they generate the same profile ID
434+
const proxy1 = new ContextProxy(mockContext)
435+
const proxy2 = new ContextProxy(mockContext)
436+
437+
// Access private method for testing
438+
const profileId1 = (proxy1 as any).generateProfileId()
439+
const profileId2 = (proxy2 as any).generateProfileId()
440+
441+
expect(profileId1).toBe(profileId2)
442+
expect(profileId1).toMatch(/^[a-f0-9]{16}$/)
443+
})
444+
445+
it("should create profile-specific secret keys", () => {
446+
const profileSpecificKey = (proxy as any).getProfileSpecificSecretKey("codeIndexQdrantApiKey")
447+
448+
expect(profileSpecificKey).toMatch(/^[a-f0-9]{16}:codeIndexQdrantApiKey$/)
449+
})
450+
451+
it("should migrate legacy secrets to profile-specific storage during initialization", async () => {
452+
// Setup mock to return legacy secret on first call, undefined on profile-specific call
453+
mockSecrets.get
454+
.mockImplementationOnce((key: string) => {
455+
// Profile-specific key call returns undefined (no existing profile-specific secret)
456+
if (key.includes(":")) return Promise.resolve(undefined)
457+
// Legacy key call returns the legacy value
458+
return Promise.resolve("legacy-qdrant-key")
459+
})
460+
.mockImplementation((key: string) => {
461+
// Subsequent calls for legacy key return the legacy value
462+
if (!key.includes(":")) return Promise.resolve("legacy-qdrant-key")
463+
return Promise.resolve(undefined)
464+
})
465+
466+
// Create new proxy to trigger initialization
467+
const newProxy = new ContextProxy(mockContext)
468+
await newProxy.initialize()
469+
470+
// Should have attempted to get profile-specific key first
471+
expect(mockSecrets.get).toHaveBeenCalledWith(expect.stringMatching(/^[a-f0-9]{16}:codeIndexQdrantApiKey$/))
472+
473+
// Should have attempted to get legacy key when profile-specific wasn't found
474+
expect(mockSecrets.get).toHaveBeenCalledWith("codeIndexQdrantApiKey")
475+
476+
// Should have stored the migrated value with profile-specific key
477+
expect(mockSecrets.store).toHaveBeenCalledWith(
478+
expect.stringMatching(/^[a-f0-9]{16}:codeIndexQdrantApiKey$/),
479+
"legacy-qdrant-key",
480+
)
481+
482+
// Should have deleted the legacy key
483+
expect(mockSecrets.delete).toHaveBeenCalledWith("codeIndexQdrantApiKey")
484+
})
485+
486+
it("should migrate legacy secrets during refreshSecrets", async () => {
487+
// Setup mock to simulate legacy secret exists
488+
mockSecrets.get.mockImplementation((key: string) => {
489+
if (key.includes(":")) return Promise.resolve(undefined) // No profile-specific secret
490+
if (key === "codeIndexQdrantApiKey") return Promise.resolve("legacy-refresh-key")
491+
return Promise.resolve(undefined)
492+
})
493+
494+
await proxy.refreshSecrets()
495+
496+
// Should have migrated the legacy secret
497+
expect(mockSecrets.store).toHaveBeenCalledWith(
498+
expect.stringMatching(/^[a-f0-9]{16}:codeIndexQdrantApiKey$/),
499+
"legacy-refresh-key",
500+
)
501+
expect(mockSecrets.delete).toHaveBeenCalledWith("codeIndexQdrantApiKey")
502+
})
503+
504+
it("should not migrate when profile-specific secret already exists", async () => {
505+
// Setup mock to return existing profile-specific secret
506+
mockSecrets.get.mockImplementation((key: string) => {
507+
if (key.includes(":codeIndexQdrantApiKey")) return Promise.resolve("existing-profile-key")
508+
return Promise.resolve(undefined)
509+
})
510+
511+
// Create new proxy to trigger initialization
512+
const newProxy = new ContextProxy(mockContext)
513+
await newProxy.initialize()
514+
515+
// Should not have attempted migration since profile-specific secret exists
516+
expect(mockSecrets.store).not.toHaveBeenCalledWith(
517+
expect.stringMatching(/^[a-f0-9]{16}:codeIndexQdrantApiKey$/),
518+
expect.any(String),
519+
)
520+
expect(mockSecrets.delete).not.toHaveBeenCalledWith("codeIndexQdrantApiKey")
521+
})
522+
523+
it("should isolate secrets between different profile environments", () => {
524+
// Mock different VSCode environments
525+
const wslEnv = {
526+
machineId: "test-machine-id",
527+
appName: "Visual Studio Code",
528+
uriScheme: "vscode-remote",
529+
}
530+
531+
const localEnv = {
532+
machineId: "test-machine-id",
533+
appName: "Visual Studio Code",
534+
uriScheme: "vscode",
535+
}
536+
537+
// Mock vscode.env for different environments
538+
vi.mocked(vscode.env).machineId = wslEnv.machineId
539+
vi.mocked(vscode.env).appName = wslEnv.appName
540+
vi.mocked(vscode.env).uriScheme = wslEnv.uriScheme
541+
542+
const wslProxy = new ContextProxy(mockContext)
543+
const wslProfileId = (wslProxy as any).generateProfileId()
544+
545+
vi.mocked(vscode.env).uriScheme = localEnv.uriScheme
546+
547+
const localProxy = new ContextProxy(mockContext)
548+
const localProfileId = (localProxy as any).generateProfileId()
549+
550+
// Profile IDs should be different for different environments
551+
expect(wslProfileId).not.toBe(localProfileId)
552+
553+
// Secret keys should be different
554+
const wslSecretKey = (wslProxy as any).getProfileSpecificSecretKey("codeIndexQdrantApiKey")
555+
const localSecretKey = (localProxy as any).getProfileSpecificSecretKey("codeIndexQdrantApiKey")
556+
557+
expect(wslSecretKey).not.toBe(localSecretKey)
558+
expect(wslSecretKey).toMatch(/^[a-f0-9]{16}:codeIndexQdrantApiKey$/)
559+
expect(localSecretKey).toMatch(/^[a-f0-9]{16}:codeIndexQdrantApiKey$/)
560+
})
561+
})
422562
})

0 commit comments

Comments
 (0)