-
Notifications
You must be signed in to change notification settings - Fork 381
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
Having folders in the binskim config causes it to fail. #8413
Conversation
…only regex that worked so far. We'll fix again later if they give us something that works better.
analyzeTargetGlob: $(Build.SourcesDirectory)/artifacts/bin/**.dll;$(Build.SourcesDirectory)/artifacts/bin/**.exe; | ||
analyzeTargetGlob: +:f|**\*.dll;+:f|**\*.exe; |
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.
This isn't the easiest way to do this because the 1ES PT doesn't have a codepath for the easiest way. 😡 This code would need to be changed: https://dev.azure.com/dnceng/_git/1ESPipelineTemplates?path=/v1/Core/Steps/Windows.SDL.Binary.Analysis.yml&version=GBmain&line=610&lineEnd=652&lineStartColumn=1&lineEndColumn=1&lineStyle=plain&_a=contents
Anyway, it should be easier to use the regex to match this stuff. This one matches paths that have artifacts\bin
in the path and end with either .dll
or .exe
. This is the suggested way to do this via the docs: https://dev.azure.com/securitytools/SecurityIntegration/_wiki/wikis/Guardian/1378/Glob-Format?anchor=optimization
I've also forced the value to be a string in YAML by wrapping it in single quotes. Just makes it easier for the YAML parser to know the data type because of the symbols and junk in there. It is just safer to do in this situation.
analyzeTargetGlob: $(Build.SourcesDirectory)/artifacts/bin/**.dll;$(Build.SourcesDirectory)/artifacts/bin/**.exe; | |
analyzeTargetGlob: +:f|**\*.dll;+:f|**\*.exe; | |
analyzeTargetGlob: 'f:regex|artifacts\\bin\\.+\.(dll|exe)$' |
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.
Per offline chat, the above suggestion doesn't work because of bugs in 1ES:
##[error]AnalyzeArgumentNoValuesException: Argument Target has no values. Check your configuration. -- Additional arguments:Microsoft.Guardian.InvalidResponseFileContentsException: InvalidResponseFileContentsException: Cannot create a response file with zero arguments. Ensure that your arguments are correctly set up.
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.
This definitely isn't the right approach but to fix this properly requires testing different combinations of parameters and 1ES PT currently doesn't allow AnalyzeTargetBinskim
, which is the easiest way to do this.
I just found out the right way. We would need to convert all the artifact publishing to use the output:
syntax. That will automatically register those items we output in the build to be scanned by binskim. I verified this looking at the dotnet/project-system repo.
This was the only regex that worked so far. We'll fix again later if they give us something that works better. Right now 9.0.1xx CI builds are blocked so this at least unblocks that.