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

fix: move undici to devDependencies. #1561

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jason3S
Copy link

@Jason3S Jason3S commented Oct 16, 2023

fixes #1560

@Jason3S Jason3S requested a review from a team as a code owner October 16, 2023 09:24
@Jason3S
Copy link
Author

Jason3S commented Oct 16, 2023

@takost and @pje,

This PR is to fix the issue where undici is getting bundled with @actions/http-client and @actions/core.

This add a large amount of bloat on actions that bundle @actions/core.

@hmurtaza7
Copy link

+1

1 similar comment
@ech018
Copy link

ech018 commented Nov 29, 2023

+1

aminya
aminya approved these changes Apr 1, 2024
@jozefizso
Copy link

Hi, what's the status of this PR?

This would fix problem with the npm audit:

undici  <=5.28.2
Undici proxy-authorization header not cleared on cross-origin redirect in fetch - https://github.com/advisories/GHSA-3787-6prv-h9w3
fix available via `npm audit fix`
node_modules/undici

1 low severity vulnerability

 @actions/[email protected]
  └─┬ @actions/[email protected]
    └── [email protected]

@jcesarmobile
Copy link

jcesarmobile commented Apr 2, 2024

As I commented on the linked issue, I don’t think it can be just moved to dev dependencies since it’s being imported in several places.

In fact it should be a dependencie in other places too, see #1685

@Jason3S
Copy link
Author

Jason3S commented Apr 3, 2024

@jozefizso,

As @jcesarmobile say, there is a dependency upon a small portion of undici. But that dependency brings in the entire thing.

I could close this since it will never land, but I am hoping someone on the GitHub team will fix the underlying issue.

@aminya
Copy link

aminya commented Apr 3, 2024

Another issue is that all the packages are transpiled to commonjs, and the TypeScript source files are not included on npm. This makes it quite hard for bundlers like Parcel that support directly using TypeScript to tree-shake such huge dependencies, so everything is included unconditionally.

@actions should strongly consider publishing the ts files in npm.

@andykenward
Copy link

Can we also bump the version of undici to the latest Security Release of ^5.28.4 ? I'm getting CodeQL security issues "Use of a broken or weak cryptographic algorithm"

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.

@actions/http-client - unnecessary dependency upon undici
7 participants