Skip to content

Commit 5c39b9d

Browse files
jrhannesrudolph
authored andcommitted
Stop leaking other provider settings (#3357)
* Stop leaking other provider settings * Also filter out leaked properties on export
1 parent 3f6432d commit 5c39b9d

File tree

5 files changed

+298
-50
lines changed

5 files changed

+298
-50
lines changed

src/core/config/ProviderSettingsManager.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import { ExtensionContext } from "vscode"
22
import { z, ZodError } from "zod"
33

4-
import { providerSettingsSchema, ApiConfigMeta } from "../../schemas"
4+
import { providerSettingsSchema, ApiConfigMeta, providerSettingsSchemaDiscriminated } from "../../schemas"
55
import { Mode, modes } from "../../shared/modes"
66
import { telemetryService } from "../../services/telemetry/TelemetryService"
77

88
const providerSettingsWithIdSchema = providerSettingsSchema.extend({ id: z.string().optional() })
9+
const discriminatedProviderSettingsWithIdSchema = providerSettingsSchemaDiscriminated.and(
10+
z.object({ id: z.string().optional() }),
11+
)
912

1013
type ProviderSettingsWithId = z.infer<typeof providerSettingsWithIdSchema>
1114

@@ -250,7 +253,11 @@ export class ProviderSettingsManager {
250253
const providerProfiles = await this.load()
251254
// Preserve the existing ID if this is an update to an existing config.
252255
const existingId = providerProfiles.apiConfigs[name]?.id
253-
providerProfiles.apiConfigs[name] = { ...config, id: config.id || existingId || this.generateId() }
256+
const id = config.id || existingId || this.generateId()
257+
258+
// Filter out settings from other providers.
259+
const filteredConfig = providerSettingsSchemaDiscriminated.parse(config)
260+
providerProfiles.apiConfigs[name] = { ...filteredConfig, id }
254261
await this.store(providerProfiles)
255262
})
256263
} catch (error) {
@@ -381,7 +388,15 @@ export class ProviderSettingsManager {
381388

382389
public async export() {
383390
try {
384-
return await this.lock(async () => providerProfilesSchema.parse(await this.load()))
391+
return await this.lock(async () => {
392+
const profiles = providerProfilesSchema.parse(await this.load())
393+
const configs = profiles.apiConfigs
394+
for (const name in configs) {
395+
// Avoid leaking properties from other providers.
396+
configs[name] = discriminatedProviderSettingsWithIdSchema.parse(configs[name])
397+
}
398+
return profiles
399+
})
385400
} catch (error) {
386401
throw new Error(`Failed to export provider profiles: ${error}`)
387402
}

src/core/config/__tests__/ProviderSettingsManager.test.ts

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,58 @@ describe("ProviderSettingsManager", () => {
247247
},
248248
}
249249

250-
expect(mockSecrets.store).toHaveBeenCalledWith(
251-
"roo_cline_config_api_config",
252-
JSON.stringify(expectedConfig, null, 2),
250+
expect(mockSecrets.store.mock.calls[0][0]).toEqual("roo_cline_config_api_config")
251+
expect(storedConfig).toEqual(expectedConfig)
252+
})
253+
254+
it("should only save provider relevant settings", async () => {
255+
mockSecrets.get.mockResolvedValue(
256+
JSON.stringify({
257+
currentApiConfigName: "default",
258+
apiConfigs: {
259+
default: {},
260+
},
261+
modeApiConfigs: {
262+
code: "default",
263+
architect: "default",
264+
ask: "default",
265+
},
266+
}),
253267
)
268+
269+
const newConfig: ProviderSettings = {
270+
apiProvider: "anthropic",
271+
apiKey: "test-key",
272+
}
273+
const newConfigWithExtra: ProviderSettings = {
274+
...newConfig,
275+
openRouterApiKey: "another-key",
276+
}
277+
278+
await providerSettingsManager.saveConfig("test", newConfigWithExtra)
279+
280+
// Get the actual stored config to check the generated ID
281+
const storedConfig = JSON.parse(mockSecrets.store.mock.lastCall[1])
282+
const testConfigId = storedConfig.apiConfigs.test.id
283+
284+
const expectedConfig = {
285+
currentApiConfigName: "default",
286+
apiConfigs: {
287+
default: {},
288+
test: {
289+
...newConfig,
290+
id: testConfigId,
291+
},
292+
},
293+
modeApiConfigs: {
294+
code: "default",
295+
architect: "default",
296+
ask: "default",
297+
},
298+
}
299+
300+
expect(mockSecrets.store.mock.calls[0][0]).toEqual("roo_cline_config_api_config")
301+
expect(storedConfig).toEqual(expectedConfig)
254302
})
255303

256304
it("should update existing config", async () => {
@@ -291,10 +339,9 @@ describe("ProviderSettingsManager", () => {
291339
},
292340
}
293341

294-
expect(mockSecrets.store).toHaveBeenCalledWith(
295-
"roo_cline_config_api_config",
296-
JSON.stringify(expectedConfig, null, 2),
297-
)
342+
const storedConfig = JSON.parse(mockSecrets.store.mock.lastCall[1])
343+
expect(mockSecrets.store.mock.lastCall[0]).toEqual("roo_cline_config_api_config")
344+
expect(storedConfig).toEqual(expectedConfig)
298345
})
299346

300347
it("should throw error if secrets storage fails", async () => {

src/exports/roo-code.d.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,22 +121,22 @@ type ProviderSettings = {
121121
unboundModelId?: string | undefined
122122
requestyApiKey?: string | undefined
123123
requestyModelId?: string | undefined
124+
fakeAi?: unknown | undefined
124125
xaiApiKey?: string | undefined
125126
groqApiKey?: string | undefined
126127
chutesApiKey?: string | undefined
127128
litellmBaseUrl?: string | undefined
128129
litellmApiKey?: string | undefined
129130
litellmModelId?: string | undefined
130-
modelMaxTokens?: number | undefined
131-
modelMaxThinkingTokens?: number | undefined
132131
includeMaxTokens?: boolean | undefined
133132
reasoningEffort?: ("low" | "medium" | "high") | undefined
134133
promptCachingDisabled?: boolean | undefined
135134
diffEnabled?: boolean | undefined
136135
fuzzyMatchThreshold?: number | undefined
137136
modelTemperature?: (number | null) | undefined
138137
rateLimitSeconds?: number | undefined
139-
fakeAi?: unknown | undefined
138+
modelMaxTokens?: number | undefined
139+
modelMaxThinkingTokens?: number | undefined
140140
}
141141

142142
type GlobalSettings = {

src/exports/types.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,22 +122,22 @@ type ProviderSettings = {
122122
unboundModelId?: string | undefined
123123
requestyApiKey?: string | undefined
124124
requestyModelId?: string | undefined
125+
fakeAi?: unknown | undefined
125126
xaiApiKey?: string | undefined
126127
groqApiKey?: string | undefined
127128
chutesApiKey?: string | undefined
128129
litellmBaseUrl?: string | undefined
129130
litellmApiKey?: string | undefined
130131
litellmModelId?: string | undefined
131-
modelMaxTokens?: number | undefined
132-
modelMaxThinkingTokens?: number | undefined
133132
includeMaxTokens?: boolean | undefined
134133
reasoningEffort?: ("low" | "medium" | "high") | undefined
135134
promptCachingDisabled?: boolean | undefined
136135
diffEnabled?: boolean | undefined
137136
fuzzyMatchThreshold?: number | undefined
138137
modelTemperature?: (number | null) | undefined
139138
rateLimitSeconds?: number | undefined
140-
fakeAi?: unknown | undefined
139+
modelMaxTokens?: number | undefined
140+
modelMaxThinkingTokens?: number | undefined
141141
}
142142

143143
export type { ProviderSettings }

0 commit comments

Comments
 (0)