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

Replace inline-css package #1021

Merged
merged 4 commits into from
Sep 7, 2023
Merged

Replace inline-css package #1021

merged 4 commits into from
Sep 7, 2023

Conversation

aoor9
Copy link
Contributor

@aoor9 aoor9 commented Aug 14, 2023

This PR would finally solve (discontinued) vm2 vulnerability by replacing the outdated inline-css with css-inline.

fixes #723, #923, #668, #691

please check #608, #732, #360

@elsheraey
Copy link

elsheraey commented Aug 17, 2023

@juandav It would be great if this got merged.

docs/mailer.md Outdated Show resolved Hide resolved
Copy link

@saratscheff saratscheff left a comment

Choose a reason for hiding this comment

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

LGTM

@AnvarAka1
Copy link

Please, review the PR. It is critical for the project, that I am working on.
Thanks beforehand!

@BramRoets
Copy link

@juandav Could this please get merged?

This package has been causing a critical security issue for almost 2 months. Many of us will have a blocked pipeline because of this dependency. Is there anything that needs to happen before this can get merged?

Screenshot 2023-08-30 at 11 02 02

@pharindoko
Copy link

@juandav @cdiaz please can you approve and merge this ?

@poojakharel
Copy link

@juandav @eduardoleal @cdiaz @p-mcgowan please approve and merge this pr

@juandav juandav merged commit 17d164b into nest-modules:master Sep 7, 2023
2 checks passed
@pharindoko
Copy link

Thanks @juandav

@SzymonGonet
Copy link

That's how poppin 💪🏻

@Stranger6667
Copy link
Contributor

Hey, css-inline author here. If you find any issues with inlining (or missing features), feel free to report them directly to the project issues list and tag me there. Hopefully, switching to css-inline would be beneficial for mailer, not only in the context of security vulnerabilities :) Cheers

@pharindoko
Copy link

@juandav
Can you create a new release that includes this PR ?

@pharindoko
Copy link

@juandav release please ! :)

@loic-combis
Copy link

Thanks for fixing this security issue!

@juandav, is there an ETA on when this will be released?

Copy link

@GMBermeo GMBermeo left a comment

Choose a reason for hiding this comment

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

Worked fine on local.

@GMBermeo
Copy link

Please, don't forget this.
Deploying something with a clear message "should not be used for production" is very bad.

@juandav

@pharindoko
Copy link

Please create a release! @juandav

@marco-verbeek
Copy link

Thank you for fixing the issue; could you please create a release for it? 🙏🏼

cc @juandav @cdiaz @dantehemerson @krzysztofmeler

@BramRoets
Copy link

I've moved away from this package and started using Nodemailer directly. (Which was pretty easy to implement.)

Unfortunately, this package is not production-ready and is not actively maintained. Security incidents are ignored for months...

@pharindoko
Copy link

@BramRoets good idea
you have some simple sample code to share to get started ?

@BramRoets
Copy link

@BramRoets good idea you have some simple sample code to share to get started ?

Here is something that should get you started. You might have to tweak the loadPartials method, or fully remove it if you are not using it.

https://gist.github.com/BramRoets/fa8d4dfe7b284654a2da528953669f74

@MallarDev
Copy link

Is there a release date for the new version, @juandav?

@kurbar
Copy link

kurbar commented Oct 11, 2023

@MallarDev If interested, I forked the repo to create a v1.9.2 release - https://www.npmjs.com/package/@kurbar/mailer
Its the latest state of master as-is and will eventually remove it after a grace period when this repo finally makes a release.

@elsheraey
Copy link

elsheraey commented Oct 23, 2023

@pharindoko You could also check this: https://medium.com/@boladebode/exploring-the-new-release-of-nest-js-version-10-and-the-migration-from-nest-modules-mailer-b80c574f89e6

Gratefully had spare time to do it just to see this beautiful status on Snyk 🗡️
image

Also -1 star to this repo. for not even having an expected release date... The release issue can be resolved but the fact that there was another issue I wanted help resolve but was afraid it will be this much delayed got me out :")

Thanks to the contributors anyways as I have used it for a while.

@pharindoko
Copy link

@pharindoko You could also check this: https://medium.com/@boladebode/exploring-the-new-release-of-nest-js-version-10-and-the-migration-from-nest-modules-mailer-b80c574f89e6

Gratefully had spare time to do it just to see this beautiful status on Snyk 🗡️ image

Also -1 star to this repo. for not even having an expected release date... The release issue can be resolved but the fact that there was another issue I wanted help resolve but was afraid it will be this much delayed got me out :")

Thanks to the contributors anyways as I have used it for a while.

nice hint! :) thanks @elsheraey

@nikolaivasilev12
Copy link

Can we please get a release on this? Its a pity to have the code merged for a almost two months now, but be missing a release on it... Our devs are being blocked by our pipelines due to it....

@Leccho
Copy link

Leccho commented Nov 1, 2023

@juandav Are you still maintaining?

@LenoM
Copy link

LenoM commented Nov 4, 2023

Adding this code may help workaround the issue while we wait for the solution:

package.json
"overrides": { "degenerator": "5.0.1" },
"dependencies": { ...

@davidulman
Copy link

@juandav Are you still maintaining?
We are waiting to the release for two months

@GreenFlag31
Copy link

Guys, just use the nodemailer without the @nestjs-modules/mailer which is not maintained anymore. This is simple and you have one dependancy less !
https://medium.com/@boladebode/exploring-the-new-release-of-nest-js-version-10-and-the-migration-from-nest-modules-mailer-b80c574f89e6

Sending a mail is a question of 10 lines of code. After having implemented it, I don't even see the added value of @nestjs-modules/mailer.

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.

#CVE-2021-23449 in the mailer module