-
Notifications
You must be signed in to change notification settings - Fork 183
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
Fix high sev security issue #8471
Conversation
@wanlwanl , I don't see the Can you add yourself to the owner of |
@wanlwanl , you need to join public Azure org to resolve the codeowner CI failures. |
|
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.
LGTM
/check-enforcer override |
Let's avoid overriding check-enforcer. The ci job should have ran but was disabled because of lack of running. I've re-enabled it so it should trigger on future changes. |
@@ -0,0 +1,3 @@ | |||
[submodule "tools/js-sdk-release-tools/deps/azure-js-dev-tools"] |
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.
Why are we using a submodule? Submodules cause all sorts of issues and we generally avoid them. If you need this dependency then we should publish the package instead of using a submodule.
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.
One example of such an issue https://github.com/Azure/azure-sdk-tools/security/dependabot/178/update-logs/370202889
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.
Thanks @weshaggard for checking it and kindly share the guidance.
Previous tool owner used a package https://www.npmjs.com/package/@ts-common/azure-js-dev-tools
but looks like no one maintain it any more, latest version still has the same security issue. So I add submodule to update the braces package. We will definitely avoid to use packages out of maintainence in the future.
No worries, I'll downgrade the package to resolve security issue.
This reverts commit c43f0da.
https://github.com/Azure/azure-sdk-tools/security/dependabot/137
https://github.com/Azure/azure-sdk-tools/security/dependabot/175