[build] Scope BinSkim scanning to build output only#10940
[build] Scope BinSkim scanning to build output only#10940jonathanpeppers merged 1 commit intomainfrom
Conversation
Add analyzeTargetGlob to restrict BinSkim to bin\Build*\ directories, excluding bin\Test*\ test assemblies. Test-built debug assemblies produce BA2021 (DoNotMarkWritableSectionsAsExecutable) false positives that add noise to every failing PR build. The 1ES template applies sdl config globally to all jobs with no per-job override mechanism, so analyzeTargetGlob is the most targeted way to exclude test output from scanning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jonathanpeppers
requested review from
grendello and
simonrozsival
as code owners
There was a problem hiding this comment.
Pull request overview
Scopes BinSkim SDL scanning in the Azure Pipelines 1ES template configuration to only scan actual product build outputs, reducing noise from test-built assemblies that can trigger false positives (e.g., BA2021).
Changes:
- Adds a BinSkim
analyzeTargetGlobfilter to include onlybin\Build*\**content. - Documents the intent inline (exclude
bin\Test*test assemblies from BinSkim scanning).
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict:
Found 1 issue:
⚠️ MSBuild targets: TheanalyzeTargetGlobsuccessfully eliminates BA2021 false positives from test assemblies underbin/Test*, but CI still fails with BA2007 (EnableCriticalCompilerWarnings) onbin/BuildRelease/net10.0/arm64/libZipSharpNative-3-3.dlland the x64 variant — these are inbin/Build*and pass through the new filter. The PR needs either a.gdnbaselinesfile to baseline BA2007 for libZipSharpNative, or the libzip CMake build needs-wd4996removed.
👍 Good targeted approach — analyzeTargetGlob is the right lever since the 1ES template doesn't support per-job SDL overrides.
Review generated by android-reviewer from review guidelines.
| binskim: | ||
| scanOutputDirectoryOnly: true | ||
| # Only scan actual build output, not test assemblies under bin/Test* | ||
| analyzeTargetGlob: +|bin\Build*\** |
There was a problem hiding this comment.
🤖 bin/Test* assemblies (fixing BA2021 noise), but CI still fails because BA2007 fires on bin/BuildRelease/net10.0/{arm64,x64}/libZipSharpNative-3-3.dll — which matches bin\Build*\**.
Options to get CI green:
- Add a
.gdnbaselinesfile to baseline BA2007 forlibZipSharpNative(the-wd4996comes from the upstream libzip CMake build and isn't trivially fixable). - Or add a second exclude pattern:
analyzeTargetGlob: +|bin\Build*\** -|**\libZipSharpNative*
Without one of these, this PR alone won't unblock CI.
There was a problem hiding this comment.
This is an improvement to not find tons of issues when MSBuild test lanes fail.
We should actually fix libZipSharp separately.
The +|bin\Build*\** glob uses BinSkim CLI prefix syntax that the 1ES Guardian wrapper does not support, causing: 'Could not parse glob pattern'. scanOutputDirectoryOnly: true already restricts scanning to the staging output directory, so the additional glob is unnecessary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The +| prefix is BinSkim CLI syntax that the 1ES Guardian wrapper cannot parse: 'Could not parse glob pattern'. Remove the +| prefix and keep the glob pattern for Guardian compatibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add analyzeTargetGlob to restrict BinSkim to bin\Build*\ directories, excluding bin\Test*\ test assemblies. Test-built debug assemblies produce BA2021 (DoNotMarkWritableSectionsAsExecutable) false positives that add noise to every failing PR build.
The 1ES template applies sdl config globally to all jobs with no per-job override mechanism, so analyzeTargetGlob is the most targeted way to exclude test output from scanning.