Skip to content

Rename BA2026.EnableAdditionalSdlSecurityChecks #586

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

Merged
merged 8 commits into from
Mar 8, 2022

Conversation

marmegh
Copy link
Contributor

@marmegh marmegh commented Feb 25, 2022

Fix #584.

EnableAdditionalSdlSecurityChecks is unnecessarily vague. Renaming to EnableMicrosoftCompilerSdlSwitch to increase clarity for customers.

@@ -373,7 +373,7 @@
"id": "NotApplicable_InvalidMetadata",
"arguments": [
"Binskim.linux-x64.dll",
"EnableAdditionalSdlSecurityChecks",
"EnableMicrosoftCompilerSdlSwitch",
Copy link
Contributor Author

@marmegh marmegh Feb 25, 2022

Choose a reason for hiding this comment

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

"EnableMicrosoftCompilerSdlSwitch",

The only change for all of these sarif results is for the rule name change. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many of these sarif file changes will become obsolete if #590 is completed first.

@marmegh
Copy link
Contributor Author

marmegh commented Feb 25, 2022

Please note, you will see a list of executables that have moved. This is due to the folder being renamed to match the new rule friendly name.


In reply to: 1051058220


In reply to: 1051058220


In reply to: 1051058220


In reply to: 1051058220

@eddynaka eddynaka mentioned this pull request Feb 28, 2022
@@ -40,7 +40,7 @@ internal static class RuleIds

public const string EnableSpectreMitigations = "BA2024";
public const string EnableShadowStack = "BA2025";
public const string EnableAdditionalSdlSecurityChecks = "BA2026";
public const string EnableMicrosoftCompilerSdlSwitch = "BA2026";
Copy link
Member

@michaelcfanning michaelcfanning Mar 2, 2022

Choose a reason for hiding this comment

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

EnableMicrosoftCompilerSdlSwitch

// Renamed from 'EnableAdditionalSdlSecurityChecks' as of 1.9.3. #Resolved

Copy link
Member

@michaelcfanning michaelcfanning left a comment

Choose a reason for hiding this comment

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

:shipit:

{
/// <summary>
/// BA2026
/// </summary>
public override string Id => RuleIds.EnableAdditionalSdlSecurityChecks;
Copy link
Contributor

@eddynaka eddynaka Mar 3, 2022

Choose a reason for hiding this comment

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

EnableAdditionalSdlSecurityChecks

We should add the deprecated name here since we already shipped a version that uses this name. #Resolved

@eddynaka
Copy link
Contributor

eddynaka commented Mar 8, 2022

can u fix the conflicts, pls?


In reply to: 1061382224


In reply to: 1061382224

@eddynaka eddynaka enabled auto-merge (squash) March 8, 2022 21:56
@eddynaka eddynaka merged commit 3dcf1fd into main Mar 8, 2022
@eddynaka eddynaka deleted the users/marmegh/renaming branch March 8, 2022 22:08
@marmegh marmegh mentioned this pull request Mar 14, 2022
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.

Rename BA2026.EnableAdditionalSdlSecurityChecks to BA2026.EnableMicrosoftCompilerSdlSwitch
3 participants