Skip to content
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

disable, restart extension host, uninstall and remove problematic extension #242249

Merged
merged 3 commits into from
Feb 28, 2025

Conversation

sandy081
Copy link
Member

disable, restart extension host, uninstall and remove problematic extension

@sandy081 sandy081 self-assigned this Feb 27, 2025
@sandy081 sandy081 enabled auto-merge (squash) February 27, 2025 22:09
@vs-code-engineering vs-code-engineering bot added this to the February 2025 milestone Feb 27, 2025
@sandy081 sandy081 closed this Feb 27, 2025
auto-merge was automatically disabled February 27, 2025 23:02

Pull request was closed

@sandy081 sandy081 reopened this Feb 27, 2025
@sandy081 sandy081 enabled auto-merge (squash) February 27, 2025 23:02
@sandy081 sandy081 merged commit 4e6f06d into main Feb 28, 2025
12 checks passed
@sandy081 sandy081 deleted the sandy081/misleading-bonobo branch February 28, 2025 07:13
Comment on lines +2982 to +2989
await this.extensionManagementService.uninstallExtensions([{
extension: e.local,
options: {
remove: true,
donotCheckDependents: true,
context: { [EXTENSION_UNINSTALL_MALICIOUS_CONTEXT]: true }
}
}]);
Copy link
Member

Choose a reason for hiding this comment

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

If this takes an array of extensions to uninstall, why aren't we using a single call to this method, instead of N calls?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you are right. I overlooked it.

Comment on lines +2990 to +2998
this.notificationService.prompt(
Severity.Warning,
nls.localize('malicious warning', "The extension '{0}' was found to be problematic and has been removed.", e.identifier.id),
[],
{
sticky: true,
priority: NotificationPriority.URGENT
}
);
Copy link
Member

Choose a reason for hiding this comment

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

Does this means that if 2 malicious extensions are removed, the user will get 2 popups?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it was same as before.

@@ -1011,9 +1013,11 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension

this.updatesCheckDelayer = new ThrottledDelayer<void>(ExtensionsWorkbenchService.UpdatesCheckInterval);
this.autoUpdateDelayer = new ThrottledDelayer<void>(1000);
this.maliciousExtensionsCheckDelayer = new ThrottledDelayer<void>(1000 * 60 * 5 /* every five minutes */);
Copy link
Member

Choose a reason for hiding this comment

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

You really just need a Delayer here. Or a regular timeout, like before.

sandy081 added a commit that referenced this pull request Feb 28, 2025
sandy081 added a commit that referenced this pull request Feb 28, 2025
sandy081 added a commit that referenced this pull request Feb 28, 2025
sandy081 added a commit that referenced this pull request Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants