Optimize Windows Defender exclusion script with native PowerShell #2930 #2939
Conversation
…ipse-platform#2930 Since the script to add an exclusion to Windows Defender seems overly complex, this script replaces the complex LINQ-based exclusion check with a more idiomatic and simpler version PowerShell implementation. Fixes eclipse-platform#2930
|
Thank you for this contribution. The changes look good too me and the script indeed looks cleaner. I'm not very familiar with Powershell and that was what I came up with after searching the internet a bit. However, I could confirm that @fdcastel's objection is correct and this script could be even simpler. |
Thanks for your feedback @HannesWell, If @fdcastel approves I can proceed to update the PR...as per your guidance ofcourse! |
|
Apologies for the delay (I was abroad). I agree with @HannesWell that the script can be simplified even further. @IamLRBA Based on my tests (and I believe @HannesWell can confirm as well), just using the call Add-MpPreference -ExclusionProcess $pathsToExcludeshould be sufficient. There is no need for any if statements or for instantiating a new |
|
Thank you for the feedback @fdcastel . You're right upon re-testing with just: |
@fdcastel, I've updated the script to simplify it as suggested. It now directly uses `Add-MpPreference -ExclusionProcess $pathsToExclude` without the extra conditionals or set creation. Let me know if anything else needs tweaking. Thanks! cc @HannesWell
|
@fdcastel, I've updated the script to simplify it as suggested. It now directly uses |
|
Looks good to me! I don’t have a Java setup here to test it out, but the changes seem fine. 🚀 👍🏻 |
There was a problem hiding this comment.
Thanks for the update.
Unfortunately this currently does not compile and a few points should be adjusted, but I made corresponding remarks below.
Furthermore, please squash your changes into one commit and force push that to your existing branch of this PR. Eventually we want to have this as one commit.
| "if($existingExclusions -eq $null) { $existingExclusions = New-Object Collections.Generic.HashSet[String] }", //$NON-NLS-1$ | ||
| "$exclusionsToAdd=[Linq.Enumerable]::ToArray([Linq.Enumerable]::Where($exclusions,[Func[object,bool]]{param($ex)!$existingExclusions.Contains($ex)}))", //$NON-NLS-1$ | ||
| "if($exclusionsToAdd.Length -gt 0){ Add-MpPreference -" + exclusionType + " $exclusionsToAdd }"); //$NON-NLS-1$ //$NON-NLS-2$ | ||
| String excludedPaths = paths.stream().map(Path::toString).map(p -> "'" + p.replace("'", "''") + "'") |
There was a problem hiding this comment.
Can you please explain the advantage of using single-quotes (and escaping them) over double-quotes in a powershell script? As far as I can tell double-quotes are not allowed in Windows paths and therefore don't have to be escaped.
| @@ -342,14 +342,12 @@ public static String createAddExclusionsPowershellCommand(String extraSeparator) | |||
| // | |||
| // For .NET's stream API called LINQ see: | |||
| // https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable | |||
There was a problem hiding this comment.
This entire comment block (starting at its first line) can be replaced by the following:
// For detailed explanations about how to add new exclusions see:
// https://learn.microsoft.com/en-us/powershell/module/defender/add-mppreference?view=windowsserver2019-ps
| "$exclusionsToAdd=[Linq.Enumerable]::ToArray([Linq.Enumerable]::Where($exclusions,[Func[object,bool]]{param($ex)!$existingExclusions.Contains($ex)}))", //$NON-NLS-1$ | ||
| "if($exclusionsToAdd.Length -gt 0){ Add-MpPreference -" + exclusionType + " $exclusionsToAdd }"); //$NON-NLS-1$ //$NON-NLS-2$ | ||
| String excludedPaths = paths.stream().map(Path::toString).map(p -> "'" + p.replace("'", "''") + "'") | ||
| .collect(Collectors.joining(',')); |
There was a problem hiding this comment.
Please keep the addition of the extraSeparator as otherwise the formatting in the preferences view doesn't look good.
| .collect(Collectors.joining(',')); | |
| .collect(Collectors.joining(',' + extraSeparator)); |
| return String.join(";", | ||
| "$pathsToExclude = @(" + excludedPaths + ")", | ||
| "Add-MpPreference -ExclusionProcess $pathsToExclude" |
There was a problem hiding this comment.
Please keep the extraSeparators here as well and keep the name of the name of the variable:
| return String.join(";", | |
| "$pathsToExclude = @(" + excludedPaths + ")", | |
| "Add-MpPreference -ExclusionProcess $pathsToExclude" | |
| return String.join(';' + extraSeparator, "$exclusions=@(" + extraSeparator + excludedPaths + ')', //$NON-NLS-1$ | |
| "Add-MpPreference -ExclusionProcess $pathsToExclude" |
Alternatively, as the script seems now easy enough, we could skip the creation of a variable and just pass the string-array directly to make it even more compact:
| return String.join(";", | |
| "$pathsToExclude = @(" + excludedPaths + ")", | |
| "Add-MpPreference -ExclusionProcess $pathsToExclude" | |
| return "Add-MpPreference -ExclusionProcess " + extraSeparator + excludedPaths; //$NON-NLS-1$ |
cc
@HannesWell
Hello,
Since the script as per @fdcastel, to add an exclusion to Windows Defender seems overly complex. Based on your concerns about the proposed
Add-MpPreference -ExclusionProcess 'C:\Program Files\DBeaver\dbeaver.exe'script, this script replaces the original complex LINQ-based exclusion check with a more idiomatic and simpler version PowerShell implementation.How It Addresses the your Concerns:
Fixes #2930