-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Modify expiration dates in .gdnbaselines #7518
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
base: main
Are you sure you want to change the base?
Conversation
Updated expiration dates for baseline results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Extends baseline suppression expiration dates to keep existing analyzer findings suppressed for longer.
- Updated multiple expirationDate values to 2026-12-31T23:59:59Z
- Did not update corresponding justification text still referencing a 180-day window
- Introduced a different timestamp format (added 'T') only for expirationDate fields
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
"expirationDate": "2026-12-31T23:59:59Z", | ||
"justification": "This error is baselined with an expiration date of 180 days from 2024-03-06 23:30:43Z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The justification text states a 180-day expiration from 2024-03-06, which would place expiration in 2024, but the updated expirationDate is 2026-12-31. Update the justification to reflect the new long-term date or adjust the expirationDate to match the stated 180-day policy.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@krwq should we update this too? Another option would be to copy the file from an internal PR which we dismiss. I think that's how we created this to begin with.
"createdDate": "2024-03-06 21:08:31Z", | ||
"expirationDate": "2024-08-23 23:30:43Z", | ||
"expirationDate": "2026-12-31T23:59:59Z", | ||
"justification": "This error is baselined with an expiration date of 180 days from 2024-03-06 23:30:43Z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The justification text states a 180-day expiration from 2024-03-06, which would place expiration in 2024, but the updated expirationDate is 2026-12-31. Update the justification to reflect the new long-term date or adjust the expirationDate to match the stated 180-day policy.
Copilot uses AI. Check for mistakes.
"createdDate": "2024-03-06 21:08:31Z", | ||
"expirationDate": "2024-08-23 23:30:43Z", | ||
"expirationDate": "2026-12-31T23:59:59Z", | ||
"justification": "This error is baselined with an expiration date of 180 days from 2024-03-06 23:30:43Z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The justification text states a 180-day expiration from 2024-03-06, which would place expiration in 2024, but the updated expirationDate is 2026-12-31. Update the justification to reflect the new long-term date or adjust the expirationDate to match the stated 180-day policy.
Copilot uses AI. Check for mistakes.
"createdDate": "2024-03-06 21:13:53Z", | ||
"expirationDate": "2024-08-23 23:30:43Z", | ||
"expirationDate": "2026-12-31T23:59:59Z", | ||
"justification": "This error is baselined with an expiration date of 180 days from 2024-03-06 23:30:43Z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The justification text states a 180-day expiration from 2024-03-06, which would place expiration in 2024, but the updated expirationDate is 2026-12-31. Update the justification to reflect the new long-term date or adjust the expirationDate to match the stated 180-day policy.
Copilot uses AI. Check for mistakes.
"createdDate": "2024-03-06 21:36:33Z", | ||
"expirationDate": "2024-08-23 23:30:43Z", | ||
"expirationDate": "2026-12-31T23:59:59Z", | ||
"justification": "This error is baselined with an expiration date of 180 days from 2024-03-06 23:30:43Z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The justification text states a 180-day expiration from 2024-03-06, which would place expiration in 2024, but the updated expirationDate is 2026-12-31. Update the justification to reflect the new long-term date or adjust the expirationDate to match the stated 180-day policy.
Copilot uses AI. Check for mistakes.
"createdDate": "2024-03-06 21:08:31Z", | ||
"expirationDate": "2024-08-23 23:30:43Z", | ||
"expirationDate": "2026-12-31T23:59:59Z", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timestamp formats are inconsistent: createdDate uses a space (YYYY-MM-DD HH:MM:SSZ) while expirationDate uses ISO 8601 with 'T'. Standardize on one format (prefer full ISO 8601 or existing project convention) to avoid parsing or readability issues. Apply consistently to all updated expirationDate entries.
Copilot uses AI. Check for mistakes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7518 +/- ##
==========================================
- Coverage 69.03% 69.01% -0.02%
==========================================
Files 1482 1482
Lines 274092 274092
Branches 28266 28266
==========================================
- Hits 189216 189175 -41
- Misses 77491 77530 +39
- Partials 7385 7387 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
"default" | ||
], | ||
"tool": "psscriptanalyzer", | ||
"ruleId": "PSAvoidUsingUsernameAndPasswordParams", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not have control over external components.
We do have control over eng/common/SetupNugetSources.ps1
that's from arcade. Maybe we should delete this one?
"memberOf": [ | ||
"default" | ||
], | ||
"tool": "binskim", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These for MKLImports are effectively an external component the way it's built today. This is a custom build of intel MKL where we cannot modify much of the pipeline. I have discussed building this in our pipeline -- we were blocked on this in the past due to the size of the toolset used to produce it -- but there may be options to build from nuget packages containing the MKL libs. Even so, we wouldn't be able to apply any compiler flags since we just link libraries provided by Intel.
We also have other components we redistribute but don't build ourselves
- OneDAL (we build OneDalNative, but many Intel onedal binaries are redistributed with it)
- libiomp (dependency of MKL)
Updated expiration dates for baseline results - those should still be suppressed in order for BinSkim to not complain. We do not have control over external components.