Skip to content

Commit 1c5e35b

Browse files
authored
fix cli ephemeral mode config leak (#4251)
* fix cli ephemeral mode config leak The ephemeral check only existed and was used in loadConfig, so any subsequent things that triggered saveConfig still caused the env vars to update the config. * add changeset
1 parent 2896316 commit 1c5e35b

File tree

3 files changed

+130
-0
lines changed

3 files changed

+130
-0
lines changed

.changeset/loose-yaks-pump.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@kilocode/cli": minor
3+
---
4+
5+
fix cli ephemeral mode config leak

cli/src/config/__tests__/persistence.test.ts

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ vi.mock("../../services/logs.js", () => ({
2424
},
2525
}))
2626

27+
// Mock env-config to control ephemeral mode behavior
28+
vi.mock("../env-config.js", async () => {
29+
const actual = await vi.importActual<typeof import("../env-config.js")>("../env-config.js")
30+
return {
31+
...actual,
32+
isEphemeralMode: vi.fn(() => false), // Default to false, tests can override
33+
}
34+
})
35+
2736
// Mock fs/promises to handle schema.json reads
2837
vi.mock("fs/promises", async () => {
2938
const actual = await vi.importActual<typeof import("fs/promises")>("fs/promises")
@@ -358,4 +367,112 @@ describe("Config Persistence", () => {
358367
expect(token).toBeNull()
359368
})
360369
})
370+
371+
describe("ephemeral mode", () => {
372+
it("should not write config file when in ephemeral mode", async () => {
373+
// Import the mocked module to control ephemeral mode
374+
const envConfig = await import("../env-config.js")
375+
vi.mocked(envConfig.isEphemeralMode).mockReturnValue(true)
376+
377+
const testConfig: CLIConfig = {
378+
version: "1.0.0",
379+
mode: "code",
380+
telemetry: false,
381+
provider: "test",
382+
providers: [
383+
{
384+
id: "test",
385+
provider: "kilocode",
386+
kilocodeToken: "env-token-should-not-be-saved",
387+
kilocodeModel: "test-model",
388+
},
389+
],
390+
}
391+
392+
// This should NOT create a file because we're in ephemeral mode
393+
await saveConfig(testConfig)
394+
395+
// Verify file was NOT created
396+
const exists = await configExists()
397+
expect(exists).toBe(false)
398+
399+
// Reset mock
400+
vi.mocked(envConfig.isEphemeralMode).mockReturnValue(false)
401+
})
402+
403+
it("should write config file when not in ephemeral mode", async () => {
404+
// Import the mocked module to control ephemeral mode
405+
const envConfig = await import("../env-config.js")
406+
vi.mocked(envConfig.isEphemeralMode).mockReturnValue(false)
407+
408+
const testConfig: CLIConfig = {
409+
version: "1.0.0",
410+
mode: "code",
411+
telemetry: false,
412+
provider: "test",
413+
providers: [
414+
{
415+
id: "test",
416+
provider: "kilocode",
417+
kilocodeToken: "real-token-should-be-saved",
418+
kilocodeModel: "test-model",
419+
},
420+
],
421+
}
422+
423+
// This should create a file because we're NOT in ephemeral mode
424+
await saveConfig(testConfig)
425+
426+
// Verify file WAS created
427+
const exists = await configExists()
428+
expect(exists).toBe(true)
429+
430+
// Verify content was written correctly
431+
const content = await fs.readFile(TEST_CONFIG_FILE, "utf-8")
432+
const parsed = JSON.parse(content)
433+
expect(parsed.providers[0].kilocodeToken).toBe("real-token-should-be-saved")
434+
})
435+
436+
it("should not persist merged config during loadConfig when in ephemeral mode", async () => {
437+
// Import the mocked module to control ephemeral mode
438+
const envConfig = await import("../env-config.js")
439+
440+
// First, create a config file while NOT in ephemeral mode
441+
vi.mocked(envConfig.isEphemeralMode).mockReturnValue(false)
442+
443+
const initialConfig: CLIConfig = {
444+
version: "1.0.0",
445+
mode: "code",
446+
telemetry: true,
447+
provider: "test",
448+
providers: [
449+
{
450+
id: "test",
451+
provider: "kilocode",
452+
kilocodeToken: "original-token-1234567890",
453+
kilocodeModel: "test-model",
454+
},
455+
],
456+
autoApproval: DEFAULT_CONFIG.autoApproval,
457+
theme: "dark",
458+
customThemes: {},
459+
}
460+
await saveConfig(initialConfig)
461+
462+
// Now switch to ephemeral mode
463+
vi.mocked(envConfig.isEphemeralMode).mockReturnValue(true)
464+
465+
// Load the config - this would normally trigger a save after merging
466+
const result = await loadConfig()
467+
expect(result.config.providers[0]).toHaveProperty("kilocodeToken", "original-token-1234567890")
468+
469+
// Verify the file still has the original content (not re-saved in ephemeral mode)
470+
const content = await fs.readFile(TEST_CONFIG_FILE, "utf-8")
471+
const parsed = JSON.parse(content)
472+
expect(parsed.providers[0].kilocodeToken).toBe("original-token-1234567890")
473+
474+
// Reset mock
475+
vi.mocked(envConfig.isEphemeralMode).mockReturnValue(false)
476+
})
477+
})
361478
})

cli/src/config/persistence.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,14 @@ export async function loadConfig(): Promise<ConfigLoadResult> {
202202
}
203203

204204
export async function saveConfig(config: CLIConfig, skipValidation: boolean = false): Promise<void> {
205+
// Don't write to disk in ephemeral mode - this prevents environment variable
206+
// values from being persisted to config.json during integration tests or
207+
// when running in ephemeral/Docker environments
208+
if (isEphemeralMode()) {
209+
logs.debug("Skipping config save in ephemeral mode", "ConfigPersistence")
210+
return
211+
}
212+
205213
try {
206214
await ensureConfigDir()
207215

0 commit comments

Comments
 (0)