fix: shell meta-interpretation and API key exposure

- Change ExecaTerminalProcess shell option from true to false so execa
  parses the command without invoking a shell, preventing shell
  metacharacter interpretation.
- Redact secret API keys in getStateToPostToWebview() before serializing
  state to the webview renderer.
- Preserve existing secret values in ProviderSettingsManager.saveConfig()
  when the webview round-trips the '__ROO_REDACTED__' sentinel.
This commit is contained in:
Jack Pippett 2026-04-28 12:36:37 -07:00
parent 3bcd7462d9
commit 4016f41c65
4 changed files with 28 additions and 6 deletions

View file

@ -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

View file

@ -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,
}
}

View file

@ -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

View file

@ -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,
}),
)
})