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(cve-2023-29827): replace EJS with Handlebars to resolve security warning #219

Merged

Conversation

KalleV
Copy link
Contributor

@KalleV KalleV commented Aug 25, 2023

Description

Migrate to handlebars to resolve security warning related to EJS dependency.

Related issues

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

dhmlau
dhmlau previously approved these changes Oct 4, 2023
@dhmlau dhmlau requested a review from achrinza October 4, 2023 19:11
@dhmlau
Copy link
Member

dhmlau commented Oct 4, 2023

@KalleV, sorry about the late response. Your changes LGTM. I'd like to have at least one more approval before merging. Thanks.

@dhmlau
Copy link
Member

dhmlau commented Oct 16, 2023

@KalleV, there's a conflict in package-lock.json. Could you please look into it? Thanks.

@KalleV
Copy link
Contributor Author

KalleV commented Oct 31, 2023

@dhmlau Certainly! I'll take care of that today.

@dhmlau
Copy link
Member

dhmlau commented Nov 7, 2023

@KalleV, sorry that it seems like your branch is out of date again, because the renovate bot has merged some dependency update PR. I've set the branch to require at least one approval now, so could you please rebase your branch and we'll make sure we'll merge your PR before the bot ones? Thanks!

@KalleV KalleV force-pushed the KalleV-replace-ejs-with-handlebars branch from f5af316 to 7220058 Compare November 8, 2023 14:43
@KalleV
Copy link
Contributor Author

KalleV commented Nov 8, 2023

@KalleV, sorry that it seems like your branch is out of date again, because the renovate bot has merged some dependency update PR. I've set the branch to require at least one approval now, so could you please rebase your branch and we'll make sure we'll merge your PR before the bot ones? Thanks!

No problem! Sure, I rebased it again.

@dhmlau
Copy link
Member

dhmlau commented Nov 8, 2023

Kicked off CI. Hopefully it'll all pass 🤞

…warning

Relates to: loopbackio/loopback-next#9867

Signed-off-by: KalleV <kvirtaneva@gmail.com>
@KalleV KalleV force-pushed the KalleV-replace-ejs-with-handlebars branch from 7220058 to 374f0a7 Compare November 9, 2023 16:40
@KalleV
Copy link
Contributor Author

KalleV commented Nov 9, 2023

Kicked off CI. Hopefully it'll all pass 🤞

😅 Another conflict on the package-lock.json. I rebased it again.

@dhmlau
Copy link
Member

dhmlau commented Nov 10, 2023

@achrinza, I'm trying to update the branch protection rules so that we can merge this PR. But multiple attempts didn't seem to work. Any idea? Thanks.

@achrinza
Copy link
Member

achrinza commented Nov 11, 2023

@dhmlau Apologies, that was on my end - I had converted this Github repository's Github Branch Protection Rules rule to the newer GitHub Repository Rules ruleset. Required to allow the OSSF Scorecard Action to have visibility into our branch protection posture without additional permissions, part of #252.

Disabling "restrict updates" seems to have fixed the problem.

edit: I've created loopbackio/security#38 to formally track this migration.

@achrinza achrinza merged commit 5b6c6cd into loopbackio:master Nov 12, 2023
12 checks passed
@achrinza
Copy link
Member

achrinza commented Nov 12, 2023

Thanks for your contributions, @KalleV! They've been merged 🎉

@KalleV KalleV deleted the KalleV-replace-ejs-with-handlebars branch November 12, 2023 17:10
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.

3 participants