[release/3.x] Fix SDL error reporting

[lpatalas]

Description

Related issue: #7616

This PR introduces following changes:

• Fail the build step when SDL reports errors - there were two problems:
  • We were shadowing $LASTEXITCODE variable by calling $LASTEXITCODE = 0 at the beginning of scripts. It should be $global:LASTEXITCODE = 0.
  • We were not checking exit code from child scripts. I fixed it by changing Write-Host to Write-Error where appropriate.
• Remove Microsoft.DotNet.XUnitRunnerUap_TemporaryKey.pfx certificate that SDL reports as vulnerability
  • This certificate is expired and probably not used anymore. We've already removed it on main.

Example build: https://dev.azure.com/dnceng/internal/_build/results?buildId=1250730
Example build that failed because of SDL error: https://dev.azure.com/dnceng/internal/_build/results?buildId=1248474

Customer Impact

Without fixing the error handling we may miss security issues reported by the SDL scanner.

Regression

There can be possible changes in behavior because we were ignoring all errors in SDL step before. Now they will fail the build.

Risk

The biggest risk is that fixing the error handling will make pipelines fail on SDL stage because of errors that we previously silently ignored.

Workarounds

If our target is to fail the build on SDL errors then I believe this the minimal amount of changes we have to make. The alternative is to keep track of the issues manually but this seems impractical in the long term.

[MattGal]

is leaving this as not $global:LASTEXITCODE intentional?

[lpatalas]

Yes, the only problem is setting $LASTEXITCODE. We could change every instance to $global:LASTEXITCODE to be on the safe side but I think it's an overkill.

[MattGal]

Signing off as changes seem reasonable but I'd like to see a run where this properly fails, since that's called out in the original issue.

[lpatalas]

@MattGal I've linked two example builds in the description.

[MattGal]

@MattGal I've linked two example builds in the description.

Thanks, I didn't realize the 2nd one was with this change.

[riarenas]

Changes look good to me. But this is a behavior change that might impact repos (we were hiding these errors when found), so I would look for approval from @mmitche or @markwilkie. They might have additional thoughts. Also, could you fill out the template in your PR description for changes to the release branches? https://github.com/dotnet/arcade/blob/main/Documentation/Policy/AskModeTellModeTemplate.md

[lpatalas]

Thanks for the comments @riarenas. I have updated the description based on the template.

[lpatalas]

This PR should be ready. It's waiting only for approvals. I also have the same changes prepared for release/5.0 on the separate branch: https://github.com/dotnet/arcade/tree/lupatala/fix-sdl-5.0. We may want to also merge them after 3.x is done.

[premun]

@mmitche @markwilkie can we get a sign-off on this backport?
(5.0 is merged already: #7661)