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.

Sign up for GitHub

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

Jump to bottom

Unpin and upgrade dependencies #263

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

Unpin and upgrade dependencies #263

wants to merge 7 commits into from

Conversation

harryzcy
Copy link
Contributor

@harryzcy harryzcy commented Oct 16, 2023 •
edited

Upgrade @babel-traverse to non-vulnerable version 7.23.2. CVE-2023-45133
Unpin dependencies to permit future upgrades, without changing prettier-plugin-sort-imports.

Fix #262

@richardjelinek-fastest
Copy link

🙏🥺

martibenn
martibenn reviewed Oct 17, 2023
View reviewed changes
src/utils/__tests__/get-code-from-ast.spec.ts
@@ -30,6 +30,7 @@ import a from 'a';
expect(format(formatted, { parser: 'babel' })).toEqual(
`// first comment
// second comment

Copy link

Choose a reason for hiding this comment

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

I don't see the point adding new lines ? Is this coming from an IDE formater ?

Copy link
Contributor Author

@harryzcy harryzcy Oct 17, 2023 •
edited

Choose a reason for hiding this comment

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

They are generated after @babel/generator >=7.19.x.

Copy link

Choose a reason for hiding this comment

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

With 7.19.3 the tests pass, but 7.19.4 they fail. Probably due to this PR: babel/babel#14979

Copy link

Choose a reason for hiding this comment

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

@Eldemarkki The PR tagged as 8.0.0-alpha.2 but the changelog saids released under 7.19.4.
Did they just messed up the release?

Copy link

Choose a reason for hiding this comment

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

seems its an unvoidable breaking change

@Julian702 Julian702 mentioned this pull request Oct 18, 2023
Merged
@harryzcy
Copy link
Contributor Author

@ayusharma Could you help with reviewing this?

@marklai1998
Copy link

@byara

@StavNoyAkur8 StavNoyAkur8 mentioned this pull request Oct 19, 2023
Closed
@acelaya acelaya mentioned this pull request Oct 19, 2023
Merged
byara
byara reviewed Oct 20, 2023
View reviewed changes
Copy link
Collaborator

@byara byara left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. Other than the new lines added to the snapshots, this looks good to me.
Can we figure out a way not to add those new lines? Otherwise, this is a breaking change

@harryzcy
Copy link
Contributor Author

Thank you for your contribution. Other than the new lines added to the snapshots, this looks good to me. Can we figure out a way not to add those new lines? Otherwise, this is a breaking change

I couldn't find any @babel/generator that controls it.

@danielemery danielemery mentioned this pull request Oct 21, 2023
Merged
@harryzcy
Copy link
Contributor Author

@byara #266 only updates @babel/traverse and leaves other dependencies untouched.

That could be the non-breaking one. And this PR could lead to a breaking change. What do you think?

@byara
Copy link
Collaborator

byara commented Oct 23, 2023

The change for babel traverse is release in v4.2.1

@harryzcy harryzcy changed the title Fix CVE-2023-45133 of @babel/traverse Unpin and upgrade dependencies Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byara byara byara left review comments

@marklai1998 marklai1998 marklai1998 approved these changes

@Eldemarkki Eldemarkki Eldemarkki left review comments

@helgefredheim helgefredheim helgefredheim approved these changes

@martibenn martibenn martibenn approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CVE-2023-45133
7 participants
@harryzcy @richardjelinek-fastest @marklai1998 @byara @helgefredheim @Eldemarkki @martibenn