Add implementation for setting to disable Github extensions#12838
Add implementation for setting to disable Github extensions#12838kevinjwang1 merged 9 commits intogoogle-gemini:mainfrom
Conversation
Summary of ChangesHello @kevinjwang1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the security posture of the application by introducing a configurable setting that allows users to explicitly disable the installation and loading of extensions originating from remote Git repositories or GitHub releases. This provides greater control over the execution of external code, addressing potential security concerns related to third-party extensions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new security setting to block the installation and loading of extensions from GitHub. The implementation correctly adds checks in both the installation and loading paths, and the associated tests and schema changes are appropriate. My review identifies a critical issue where the implementation is overly broad, blocking all git sources instead of just GitHub, which contradicts the feature's description. I've provided suggestions to correct this logic. I also noted that this security check is duplicated and recommended refactoring it into a helper method for better maintainability.
| if ( | ||
| (installMetadata.type === 'git' || | ||
| installMetadata.type === 'github-release') && | ||
| this.settings.security?.blockGithubExtensions | ||
| ) { | ||
| throw new Error( | ||
| 'Installing extensions from remote sources is disallowed by your current settings.', | ||
| ); | ||
| } |
There was a problem hiding this comment.
The current implementation incorrectly blocks all extensions of type git, not just those from GitHub. This contradicts the setting's name (blockGithubExtensions) and description, which specify blocking extensions only from GitHub. A user might want to block public GitHub extensions but still allow installations from a private Git server. The check should be more specific to GitHub sources. I've also updated the error message to be more specific.
Additionally, this security check is duplicated in the loadExtension method. Consider extracting this logic into a private helper method to improve maintainability and reduce risk.
| if ( | |
| (installMetadata.type === 'git' || | |
| installMetadata.type === 'github-release') && | |
| this.settings.security?.blockGithubExtensions | |
| ) { | |
| throw new Error( | |
| 'Installing extensions from remote sources is disallowed by your current settings.', | |
| ); | |
| } | |
| const isGithubSource = | |
| installMetadata.type === 'github-release' || | |
| (installMetadata.type === 'git' && | |
| !!tryParseGithubUrl(installMetadata.source)); | |
| if (isGithubSource && this.settings.security?.blockGithubExtensions) { | |
| throw new Error( | |
| 'Installing extensions from GitHub is disallowed by your current settings.', | |
| ); | |
| } |
| if ( | ||
| (installMetadata?.type === 'git' || | ||
| installMetadata?.type === 'github-release') && | ||
| this.settings.security?.blockGithubExtensions | ||
| ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Similar to the installOrUpdateExtension method, this check incorrectly blocks all git sources instead of just GitHub sources. This should be updated to specifically check for GitHub URLs.
As mentioned in the other comment, this duplicated logic should be extracted into a shared private helper method to ensure consistency and maintainability of this security feature.
| if ( | |
| (installMetadata?.type === 'git' || | |
| installMetadata?.type === 'github-release') && | |
| this.settings.security?.blockGithubExtensions | |
| ) { | |
| return null; | |
| } | |
| const isGithubSource = | |
| installMetadata && | |
| (installMetadata.type === 'github-release' || | |
| (installMetadata.type === 'git' && | |
| !!tryParseGithubUrl(installMetadata.source))); | |
| if (isGithubSource && this.settings.security?.blockGithubExtensions) { | |
| return null; | |
| } |
2 participants
Summary
Add a new setting to disable installing and loading extensions from github.
Details
Uses the install metadata to check if a installed extension is from local or remote sources. Rejects installing the extension if its from a remote source. If a remote extension is already installed, the extension manager will reject loading that extension.
Related Issues
Resolves #12557