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

tech story: Update dompurify and jsPDF to fix dependabot alert #10955

Merged
merged 4 commits into from
Sep 18, 2024

Conversation

coliu-akamai
Copy link
Contributor

@coliu-akamai coliu-akamai commented Sep 17, 2024

Description 📝

Fixes dependabot alert here: https://github.com/linode/manager/security/dependabot/121
See #10953 for context

Changes 🔄

  • Updates package.json to make dompurify dependency v3.1.6
  • also takes the change from the linked PR to remove dompurify v2.4.7
  • updates jsPDF to newest version (2.5.2) since they address the dompurify issue there

How to test 🧪

  • confirm github actions pass
  • confirm running yarn install doesn't change the yarn.lock file
  • *** confirm no regression in invoice pdf generation

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@coliu-akamai coliu-akamai marked this pull request as ready for review September 17, 2024 16:44
@coliu-akamai coliu-akamai requested a review from a team as a code owner September 17, 2024 16:44
@coliu-akamai coliu-akamai requested review from dwiley-akamai, cpathipa and bnussman-akamai and removed request for a team September 17, 2024 16:44
@coliu-akamai coliu-akamai self-assigned this Sep 17, 2024
@coliu-akamai coliu-akamai added the Dependencies Pull requests that update a dependency file label Sep 17, 2024
Copy link

github-actions bot commented Sep 17, 2024

Coverage Report:
Base Coverage: 86.93%
Current Coverage: 86.93%

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

GHA ✅ (the two failing E2E tests seem to be flakes)
No issues with yarn.lock locally ✅

@coliu-akamai
Copy link
Contributor Author

update: saw that a jsPDF update just came out to address the dompurify security concern - v2.5.2. I've updated jsPDF on my branch + checked that all tests pass. There shouldn't be any breaking changes based on the release notes, but will be trying to test with generating invoices as well before pushing it up!

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you 📦

@coliu-akamai
Copy link
Contributor Author

@bnussman-akamai @dwiley-akamai just pushed up a change to update jsPDF as well - would you be able to rereview with a focus that there are no regressions in invoice generation 😅

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Confirmed yarn.lock looks good and PDF invoices are also good ✅

@coliu-akamai coliu-akamai changed the title tech story: Update dompurify to fix dependabot alert tech story: Update dompurify and jsPDF to fix dependabot alert Sep 18, 2024
Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

Invoice PDFs look good to me as well 🚢

@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Sep 18, 2024
@coliu-akamai coliu-akamai merged commit d07788d into linode:develop Sep 18, 2024
18 of 20 checks passed
@coliu-akamai coliu-akamai deleted the update-dompurify-again branch September 18, 2024 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants