-
Notifications
You must be signed in to change notification settings - Fork 1
[EVAL] API key from Env Var support #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EVAL] API key from Env Var support #41
Conversation
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
…ssage Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
…viders Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
OPENAI_API_KEY env var Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
whole process.env into webview Signed-off-by: Geoff Wilson <[email protected]>
of api key env var; use constants for env var keys. Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
errant keys in test file Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
vercel copy/pasta Signed-off-by: Geoff Wilson <[email protected]>
Signed-off-by: Geoff Wilson <[email protected]>
| switch (apiConfiguration.apiProvider) { | ||
| case "openrouter": | ||
| if (!apiConfiguration.openRouterApiKey) { | ||
| if (!(apiConfiguration.openRouterApiKey || apiConfiguration.openRouterConfigUseEnvVars)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation updates for env var support are incomplete. While this PR correctly updates validation for openrouter, glama, unbound, requesty, litellm, anthropic, gemini, openai-native, mistral, and openai, it's missing updates for several other providers that now support env vars:
cerebras(line 131-135)fireworks(line 136-140)io-intelligence(line 141-145)featherless(line 146-150)vercel-ai-gateway(line 155-159)huggingface(line 123-130)deepinfra(not in validation at all)doubao(not in validation at all)moonshot(not in validation at all)zai(not in validation at all)sambanova(not in validation at all)
These providers all have ConfigUseEnvVars flags added in the schema and are used in buildApiHandler, but their validation logic still only checks for the direct API key. This creates an inconsistency where users could enable env var mode but the validation would still fail.
| FIREWORKS: 'FIREWORKS_API_KEY', | ||
| HUGGING_FACE: 'HUGGINGFACE_API_KEY', | ||
| IO_INTELLIGENCE: 'IOINTELLIGENCE_API_KEY', | ||
| MOONSHOOT: 'MOONSHOT_API_KEY', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in constant name: MOONSHOOT should be MOONSHOT (missing 'T'). This typo is then propagated to the usage in buildApiHandler at line 165.
| MOONSHOOT: 'MOONSHOT_API_KEY', | |
| MOONSHOT: 'MOONSHOT_API_KEY', |
| return new QwenCodeHandler(options) | ||
| case "moonshot": | ||
| if (options.moonshotConfigUseEnvVars) { | ||
| options.moonshotApiKey = getEnvVar(API_KEYS.MOONSHOOT, options.moonshotApiKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This references the typo MOONSHOOT from constants.ts (line 25). Should be API_KEYS.MOONSHOT to match the corrected constant name.
| options.moonshotApiKey = getEnvVar(API_KEYS.MOONSHOOT, options.moonshotApiKey) | |
| options.moonshotApiKey = getEnvVar(API_KEYS.MOONSHOT, options.moonshotApiKey) |
| AwsBedrockHandler: vi.fn().mockImplementation((options) => ({ provider: "bedrock", options })), | ||
| VertexHandler: vi.fn().mockImplementation((options) => ({ provider: "vertex", options })), | ||
| AnthropicVertexHandler: vi.fn().mockImplementation((options) => ({ provider: "anthropic-vertex", options })), | ||
| OllamaHandler: vi.fn().mockImplementation((options) => ({ provider: "ollama", options })), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test file is missing a mock for NativeOllamaHandler which is used in buildApiHandler at line 138. This will cause the test to fail when it tries to import the actual handler.
| OllamaHandler: vi.fn().mockImplementation((options) => ({ provider: "ollama", options })), | |
| OllamaHandler: vi.fn().mockImplementation((options) => ({ provider: "ollama", options })), | |
| NativeOllamaHandler: vi.fn().mockImplementation((options) => ({ provider: "ollama", options })), |
| const envVarExists = (window as any).ENV_VAR_EXISTS || {} | ||
| const apiKeyEnvVarExists = !!envVarExists[apiKeyEnvVar] | ||
| const [useEnvVar, setUseEnvVar] = useState(configUseEnvVars && apiKeyEnvVarExists) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useEnvVar state is initialized from props but doesn't update when configUseEnvVars or apiKeyEnvVarExists change. If the environment variable becomes available after component mount (e.g., user sets it and reloads settings), or if configUseEnvVars changes externally, the checkbox state won't reflect the change. Consider using useEffect to sync state with props:
| const envVarExists = (window as any).ENV_VAR_EXISTS || {} | |
| const apiKeyEnvVarExists = !!envVarExists[apiKeyEnvVar] | |
| const [useEnvVar, setUseEnvVar] = useState(configUseEnvVars && apiKeyEnvVarExists) | |
| const envVarExists = (window as any).ENV_VAR_EXISTS || {} | |
| const apiKeyEnvVarExists = !!envVarExists[apiKeyEnvVar] | |
| const [useEnvVar, setUseEnvVar] = useState(configUseEnvVars && apiKeyEnvVarExists) | |
| useEffect(() => { | |
| setUseEnvVar(configUseEnvVars && apiKeyEnvVarExists) | |
| }, [configUseEnvVars, apiKeyEnvVarExists]) |
|
|
||
| const huggingFaceSchema = baseProviderSettingsSchema.extend({ | ||
| huggingFaceApiKey: z.string().optional(), | ||
| huggingFaceConfigUseEnvVars: z.boolean().optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace after the comma. This should be removed for consistency with the rest of the file.
| huggingFaceConfigUseEnvVars: z.boolean().optional(), | |
| huggingFaceConfigUseEnvVars: z.boolean().optional(), |
|
|
||
| const sambaNovaSchema = apiModelIdProviderModelSchema.extend({ | ||
| sambaNovaApiKey: z.string().optional(), | ||
| sambaNovaConfigUseEnvVars: z.boolean().optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace after the comma. This should be removed for consistency.
| sambaNovaConfigUseEnvVars: z.boolean().optional(), | |
| sambaNovaConfigUseEnvVars: z.boolean().optional(), |
|
|
||
| const featherlessSchema = apiModelIdProviderModelSchema.extend({ | ||
| featherlessApiKey: z.string().optional(), | ||
| featherlessConfigUseEnvVars: z.boolean().optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace after the comma. This should be removed for consistency.
| featherlessConfigUseEnvVars: z.boolean().optional(), | |
| featherlessConfigUseEnvVars: z.boolean().optional(), |
| API_KEY_ENV_VAR_NAMES.forEach((envVar) => { | ||
| const exists = !!process.env[envVar] | ||
| result[envVar] = exists | ||
| if (exists) { | ||
| console.log(`[ClineProvider] Found environment variable: ${envVar}`) | ||
| } | ||
| }) | ||
| return result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: This logs the names of environment variables that exist to the console. While it doesn't log the actual values, revealing which API keys are configured could be considered an information disclosure issue in shared development environments or when console logs are captured. Consider removing this logging or making it debug-only:
| API_KEY_ENV_VAR_NAMES.forEach((envVar) => { | |
| const exists = !!process.env[envVar] | |
| result[envVar] = exists | |
| if (exists) { | |
| console.log(`[ClineProvider] Found environment variable: ${envVar}`) | |
| } | |
| }) | |
| return result | |
| API_KEY_ENV_VAR_NAMES.forEach((envVar) => { | |
| const exists = !!process.env[envVar] | |
| result[envVar] = exists | |
| }) |
🧪 Evaluation PR for Testing
This is an automated test PR created for evaluation purposes.
Related GitHub Issue
Closes: #4337
Description
This PR adds support for obtaining API keys from the environment. A checkbox is added to each configuration that allows inputting an API key -- when checked, a given environment variable (eg, OPENAI_API_KEY, etc) is consulted for the key and the user is not required to provide one in the configuration. Env Var key is predictable but not editable.
Test Procedure
I tested this using VS Code run configurations, where the expected environment variable could be provided or omitted.
Pre-Submission Checklist
A documentation update is advised, to describe environment variable usage to supply API keys.
Summary
Adds support for using environment variables for API keys across multiple providers, updates UI components to reflect this, and enhances testing and translation capabilities.