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

Fill in .gdnsuppress to fix internal build #1457

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

dagood
Copy link
Member

@dagood dagood commented Dec 20, 2024

Add more entries to .config/guardian/.gdnsuppress to match the current list of false positive credscan detections.

We are set up to use the TSA service to automatically use AzDO work items to handle credscan suppression, but it doesn't seem to be working now. In the past, there has also been an outage where work items weren't created, so we had to use the suppression file to unblock our builds. I think we should stop depending on the TSA workflow and instead simply update the .config/guardian/.gdnsuppress all the time. This way, we get rid of the seemingly fragile TSA dependency and avoid time spent diagnosing issues with it.

Closing AzDO work items in the right way to suppress issues is also, in my opinion, fiddlier than it needs to be, time consuming, and has a more difficult historical trail to follow than commits to a file do.


If a build hits credscan issues, it produces a suppressions file here:

image

As of writing, that file contains a warning:

"hydrationStatus": "This file contains identifying data. It is **NOT** safe to check into your repo. To dehydrate this file, run `guardian dehydrate --help` and follow the guidance."

Getting a setup working to dehydrate the file is tedious and is a reason I've avoided this process in the past. However, I checked with the 1ES PT team and they are talking with the Guardian team to update this message. It is safe to check the hydrated file into the repo:
https://teams.microsoft.com/l/message/19:[email protected]/1734637010168?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=886b1d22-e648-4492-af5f-6fbe81e73d4a&parentMessageId=1734637010168&teamName=1ES%20Pipeline%20Templates%20(1ES%20PT)&channelName=Support%20-%20General&createdTime=1734637010168
The 1ES PT team is contacting the Guardian team to get that message updated.

So, the new workflow to resolve a false positive and unblock the build is to grab that artifact and submit a PR with its results copied into this file.

I don't know yet whether a TSA-opened work item associated with a false positive will automatically be closed when the PR is merged and an internal build runs on the result. (I'm hoping so. But it won't block our build if they don't, and we can address that later.)

Test internal build that passed with this PR: https://dev.azure.com/dnceng/internal/_build/results?buildId=2605810&view=results

@karianna karianna merged commit 2f0804d into microsoft/main Dec 20, 2024
31 checks passed
@dagood dagood deleted the dev/dagood/more-suppress-credscan branch December 20, 2024 22:25
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.

2 participants