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

search-ui-react: Resolve Vulnerabilities #480

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

mkouzel-yext
Copy link
Contributor

@mkouzel-yext mkouzel-yext commented Nov 22, 2024

Regenerates test-site/package-lock.json to include the safe dependencies.

J=VULN-39418, VULN-39419
TEST=none

Saw that babel imports in package-lock.json and test-site/package-lock.json were for versions at or above the safe dependency.

Regenerates package-lock.json to include the safe dependencies.

J=VULN-39418, VULN-39419
TEST=none

Saw that babel imports in package-lock.json and test-site/package-lock.json
were for versions at or above the safe dependency.
@mkouzel-yext mkouzel-yext requested a review from a team as a code owner November 22, 2024 20:09
@coveralls
Copy link

coveralls commented Nov 22, 2024

Coverage Status

coverage: 85.38%. remained the same
when pulling ae9dcbe on hotfix/v1.6.8
into 109b443 on main.

@Fondryext
Copy link
Contributor

To make sure I understand, this PR is just regenerating test-site's package-lock to get the package updates? Your comments mention both package-locks but the base one doesn't look like it has any changes other than incrementing the version

@mkouzel-yext
Copy link
Contributor Author

Yes, I just incremented the package version in ./package.json. If the only change to package-lock.json was its package version then that's fine - to be clear I basically just incremented the package version and ran rm -rf node_modules package-lock.json && npm i in . and ./test-site.

@Fondryext
Copy link
Contributor

Since we're already doing this change, could you see if it would be easy to include https://yexttest.atlassian.net/browse/VULN-38477 as part of this? To reduce the number of patch versions. Thanks!

@mkouzel-yext
Copy link
Contributor Author

mkouzel-yext commented Nov 27, 2024

I looked through test-site/package-lock.json and it now has a safe @babel/traverse version, so that other ticket should be fixed by this PR as well. I expect the issue was that the package-lock file hadn't been regenerated in test-site.

@vijay267
Copy link

I'm also a little confused by this change? Shouldn't there be some other changes upgrading the dependency causing a vulnerability like Jacob said? The test site is just a test site to be able to test search-ui-react so I'd expect the main repo's package.json to change more.

Also separately not sure why but seems like some of the github checks are failing.

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "@yext/search-ui-react",
"version": "1.6.7",
"version": "1.6.8",
"description": "A library of React Components for powering Yext Search integrations",
"author": "[email protected]",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should be a separate change, but shouldn't this say [email protected]?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure we might as well do it here, since otherwise it'd be yet another patch version lol.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in new commit

@Fondryext
Copy link
Contributor

I'm also a little confused by this change? Shouldn't there be some other changes upgrading the dependency causing a vulnerability like Jacob said? The test site is just a test site to be able to test search-ui-react so I'd expect the main repo's package.json to change more.

Also separately not sure why but seems like some of the github checks are failing.

To be clear @vijay267 , what happened was the normal package, the normal package-lock, and the test site package, all got updated in the previous version, but not the test site package lock. So that's what this change is.

@vijay267
Copy link

vijay267 commented Dec 9, 2024

Ah got it. That makes sense then. Thanks for the explanation Jacob!

Copy link
Contributor

github-actions bot commented Dec 9, 2024

Current unit coverage is 92.12481426448737%
Current visual coverage is 79.01234567901234%
Current combined coverage is 92.62376237623762%

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.

4 participants