Skip to content

Conversation

@suprjinx
Copy link

@suprjinx suprjinx commented Apr 29, 2025

Context

This PR will add support for specifying an ENV VAR name for API keys, which is useful in cases where the secret is more dynamic and/or is injected into the dev environment.

Implementation

The high level approach was to add an additional text field to the ApiOptions component for specifying the the ENV VAR name which holds the api key. API client will fetch the API key from this ENV VAR when the name is provided.

Screenshots

Before
image
After
image
image

How to Test

  • Export an API key to an environment variable, and then start vscode
  • Configure the API options to reference this ENV VAR and remove API key
  • Expect the API to work

Get in Touch


Important

Adds support for using environment variables to store API keys, updating backend logic, schemas, and UI components to accommodate this feature.

  • Behavior:
    • Adds support for specifying environment variable names for API keys in buildApiHandler() in index.ts.
    • Updates getEnvVar() in index.ts to fetch API keys from environment variables if specified.
    • Modifies validateApiConfiguration() in validate.ts to check for either API key or environment variable name.
  • Schemas:
    • Adds apiKeyEnvVar fields to various provider schemas in schemas/index.ts.
  • UI Components:
    • Updates components like Anthropic.tsx, DeepSeek.tsx, Gemini.tsx, etc., to include input fields for environment variable names.
    • Updates settings.json for new translations related to environment variable support.

This description was created by Ellipsis for 1b9fcf3b5bbaa29d72d69fc66fc3b9694cabff37. You can customize this summary. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented Apr 29, 2025

⚠️ No Changeset found

Latest commit: 6e0c7221bcbd93e63adf8c8937bd13f61d67d4fe

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap Apr 29, 2025
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of this - let us know when it's ready for review!

@suprjinx suprjinx marked this pull request as ready for review May 9, 2025 17:15
@suprjinx suprjinx requested a review from cte as a code owner May 9, 2025 17:15
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels May 9, 2025
@suprjinx
Copy link
Author

suprjinx commented May 9, 2025

I like the idea of this - let us know when it's ready for review!

Thanks @mrubens! Ready for review although I did not update i18n except EN (do you have a tool to aid translations?)

Check warning

Code scanning / CodeQL

Prototype-polluting function Medium

The property chain
here
is recursively assigned to
current
without guarding against prototype pollution.
@suprjinx
Copy link
Author

Thanks @mrubens! Ready for review although I did not update i18n except EN (do you have a tool to aid translations?)

I'm adding a "--create-missing" option to the find-missing-translations.js script -- this will regenerate the target locale with the keys, so it's easier to get a bunch of keys translated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right translation key

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for catching -- fixed here and other providers

Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UX-wise, I think this might be better as some sort of toggle between the API key input and the env var configuration. I need to think a little bit about the best way.

@suprjinx
Copy link
Author

UX-wise, I think this might be better as some sort of toggle between the API key input and the env var configuration. I need to think a little bit about the best way.

@mrubens what do you think of just a checkbox with label "Read API Key from environment variable OPEN_AI_API_KEY" and so on. IE, we just specify the environment var we'll check instead of collecting it through a form element. Might make things simpler.

@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap May 20, 2025
@suprjinx
Copy link
Author

UX-wise, I think this might be better as some sort of toggle between the API key input and the env var configuration. I need to think a little bit about the best way.

@mrubens I made a field toggle for the Anthropic provider (screenshots of two states in PR description) -- let me know what you think

@hannesrudolph hannesrudolph moved this from PR [Needs Review] to TEMP in Roo Code Roadmap May 26, 2025
@daniel-lxs daniel-lxs moved this from TEMP to PR [Needs Review] in Roo Code Roadmap May 27, 2025
@suprjinx suprjinx closed this May 27, 2025
@suprjinx suprjinx force-pushed the add_api_key_env_vars branch from 75aaadb to 4ea7562 Compare May 27, 2025 17:52
@github-project-automation github-project-automation bot moved this from PR [Pre Approval Review] to Done in Roo Code Roadmap May 27, 2025
@github-project-automation github-project-automation bot moved this from Needs Preliminary Review to Done in Roo Code Roadmap May 27, 2025
@suprjinx
Copy link
Author

will reopen after some refactoring

Signed-off-by: Geoff Wilson <[email protected]>
@suprjinx
Copy link
Author

Reopen with rebase and new approach

@suprjinx suprjinx reopened this May 27, 2025
@dosubot dosubot bot removed the size:L This PR changes 100-499 lines, ignoring generated files. label May 27, 2025
@github-project-automation github-project-automation bot moved this from Done to Triage in Roo Code Roadmap May 27, 2025
@github-project-automation github-project-automation bot moved this from Done to New in Roo Code Roadmap May 27, 2025
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 27, 2025
@hannesrudolph hannesrudolph added Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. and removed PR - Needs Preliminary Review labels May 27, 2025
@hannesrudolph
Copy link
Collaborator

@suprjinx After reviewing your approach, I think we’d benefit from discussing this idea through an issue first, following our Issue-First approach. This helps us align on the scope, complexity, and broader applicability before investing effort into implementation.

The current solution, focused specifically on the Anthropic provider, feels like it addresses a fairly niche use case. Opening an issue to discuss it first would help ensure we’re aligned on whether this complexity fits well into Roo’s overall direction and provider integration strategy.

Would you mind creating an issue outlining your motivation and providing some context around the broader benefits? We can quickly review and agree on a path forward from there.

@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap May 28, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap May 28, 2025
@suprjinx
Copy link
Author

suprjinx commented Jun 2, 2025

@suprjinx After reviewing your approach, I think we’d benefit from discussing this idea through an issue first, following our Issue-First approach. This helps us align on the scope, complexity, and broader applicability before investing effort into implementation.

The current solution, focused specifically on the Anthropic provider, feels like it addresses a fairly niche use case. Opening an issue to discuss it first would help ensure we’re aligned on whether this complexity fits well into Roo’s overall direction and provider integration strategy.

Would you mind creating an issue outlining your motivation and providing some context around the broader benefits? We can quickly review and agree on a path forward from there.

Thanks for feedback, @hannesrudolph -- I can't create a New Feature issue, it just directs to the discussions. This discussion was the starting point: #2057.

The intention was to implement for all providers supporting api keys, not just Anthropic -- was just trying to get a prototype nailed down.

@hannesrudolph
Copy link
Collaborator

@suprjinx After reviewing your approach, I think we’d benefit from discussing this idea through an issue first, following our Issue-First approach. This helps us align on the scope, complexity, and broader applicability before investing effort into implementation.
The current solution, focused specifically on the Anthropic provider, feels like it addresses a fairly niche use case. Opening an issue to discuss it first would help ensure we’re aligned on whether this complexity fits well into Roo’s overall direction and provider integration strategy.
Would you mind creating an issue outlining your motivation and providing some context around the broader benefits? We can quickly review and agree on a path forward from there.

Thanks for feedback, @hannesrudolph -- I can't create a New Feature issue, it just directs to the discussions. This discussion was the starting point: #2057.

The intention was to implement for all providers supporting api keys, not just Anthropic -- was just trying to get a prototype nailed down.

Pasted_Image_2025-06-02__4_06 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants