diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 6088bd68fe..dd2caf7fbe 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -368,6 +368,19 @@ export class ProviderSettingsManager { typeof config.apiProvider === "string" && isRetiredProvider(config.apiProvider) ? providerSettingsWithIdSchema.passthrough().parse(config) : discriminatedProviderSettingsWithIdSchema.parse(config) + + // Preserve existing secret values when the webview sends back the + // "__ROO_REDACTED__" sentinel (secrets are redacted in the webview state + // to prevent API key exposure). + const existingConfig = providerProfiles.apiConfigs[name] + if (existingConfig) { + for (const key of Object.keys(filteredConfig)) { + if (isSecretStateKey(key) && (filteredConfig as any)[key] === "__ROO_REDACTED__") { + ;(filteredConfig as any)[key] = (existingConfig as any)[key] + } + } + } + providerProfiles.apiConfigs[name] = { ...filteredConfig, id } await this.store(providerProfiles) return id diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 1106d34005..78e2f4433e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -97,6 +97,7 @@ import { Task } from "../task/Task" import { webviewMessageHandler } from "./webviewMessageHandler" import type { ClineMessage, TodoItem } from "@roo-code/types" +import { isSecretStateKey } from "@roo-code/types" import { readApiMessages, saveApiMessages, saveTaskMessages, TaskHistoryStore } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" @@ -2468,9 +2469,17 @@ export class ClineProvider ) } + // Redact secrets before sending to webview to prevent API key exposure. + const redactedApiConfiguration = { ...providerSettings } + for (const key of Object.keys(redactedApiConfiguration)) { + if (isSecretStateKey(key) && typeof (redactedApiConfiguration as any)[key] === "string") { + ;(redactedApiConfiguration as any)[key] = "__ROO_REDACTED__" + } + } + // Return the same structure as before. return { - apiConfiguration: providerSettings, + apiConfiguration: redactedApiConfiguration, lastShownAnnouncementId: stateValues.lastShownAnnouncementId, customInstructions: stateValues.customInstructions, apiModelId: stateValues.apiModelId, @@ -2571,7 +2580,7 @@ export class ClineProvider maxGitStatusFiles: stateValues.maxGitStatusFiles ?? 0, taskSyncEnabled, imageGenerationProvider: stateValues.imageGenerationProvider, - openRouterImageApiKey: stateValues.openRouterImageApiKey, + openRouterImageApiKey: stateValues.openRouterImageApiKey ? "__ROO_REDACTED__" : undefined, openRouterImageGenerationSelectedModel: stateValues.openRouterImageGenerationSelectedModel, } } diff --git a/src/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index cc2af93802..82d39f0bfd 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -40,7 +40,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess { this.isHot = true this.subprocess = execa({ - shell: BaseTerminal.getExecaShellPath() || true, + shell: BaseTerminal.getExecaShellPath() || false, cwd: this.terminal.getCurrentWorkingDirectory(), all: true, // Ignore stdin to ensure non-interactive mode and prevent hanging diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index 5f0a21869e..500c67260b 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -63,7 +63,7 @@ describe("ExecaTerminalProcess", () => { const execaMock = vitest.mocked(execa) expect(execaMock).toHaveBeenCalledWith( expect.objectContaining({ - shell: true, + shell: false, cwd: "/test/cwd", all: true, env: expect.objectContaining({ @@ -105,13 +105,13 @@ describe("ExecaTerminalProcess", () => { ) }) - it("should fall back to shell=true when execaShellPath is undefined", async () => { + it("should fall back to shell=false when execaShellPath is undefined", async () => { BaseTerminal.setExecaShellPath(undefined) await terminalProcess.run("echo test") const execaMock = vitest.mocked(execa) expect(execaMock).toHaveBeenCalledWith( expect.objectContaining({ - shell: true, + shell: false, }), ) })