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

Patch Updating Postcss to solve CVE vulnerabity #1489

Merged
merged 6 commits into from
Nov 10, 2023
Merged

Conversation

rv0lt
Copy link
Member

@rv0lt rv0lt commented Nov 8, 2023

Read this before submitting the PR

  1. Always create a Draft PR first
  2. Go through sections 1-5 below, fill them in and check all the boxes
  3. Make sure that the branch is updated; if there's an "Update branch" button at the bottom of the PR, rebase or update branch.
  4. When all boxes are checked, information is filled in, and the branch is updated: mark as Ready For Review and tag reviewers (top right)
  5. Once there is a submitted review, implement the suggestions (if reasonable, otherwise discuss) and request an new review.

If there is a field which you are unsure about, enter the edit mode of this description or go to the PR template; There are invisible comments providing descriptions which may be of help.

1. Description / Summary

Patch update of postcss from 8.4.28 to 8.4.31 to solve related CVE


PostCSS is Node package (used for the web functionalities) to parse JavaScript code into CSS. we use it for the styles.

The changes in the package are minor, therefore nothing should get affected

Changelog

The update was done execution inside /dds_web/static:

npm update postcss --save

Which safely identified the version 8.4.31 to update

2. Jira task / GitHub issue

https://scilifelab.atlassian.net/jira/software/projects/DDS/boards/13?selectedIssue=DDS-1812

3. Type of change

What type of change(s) does the PR contain?

Check the relevant boxes below. For an explanation of the different sections, enter edit mode of this PR description template.

  • New feature
    • Breaking: Why / How? Add info here.
    • Non-breaking
  • Database change: Remember the to include a new migration version, or explain here why it's not needed.
  • Bug fix
  • Security Alert fix
    • Package update
      • Major version update
  • Documentation
  • Workflow
  • Tests only

4. Additional information

5. Actions / Scans

Check the boxes when the specified checks have passed.

For information on what the different checks do and how to fix it if they're failing, enter edit mode of this description or go to the PR template.

  • Black
  • Prettier
  • Yamllint
  • Tests
  • CodeQL
  • Trivy
  • Snyk

@rv0lt rv0lt marked this pull request as ready for review November 8, 2023 13:24
@rv0lt rv0lt requested a review from a team November 8, 2023 13:33
Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Merging #1489 (d8a39ca) into dev (e48ce8a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev    #1489   +/-   ##
=======================================
  Coverage   91.48%   91.48%           
=======================================
  Files          29       29           
  Lines        4617     4617           
=======================================
  Hits         4224     4224           
  Misses        393      393           

@rv0lt rv0lt self-assigned this Nov 9, 2023
@i-oden
Copy link
Member

i-oden commented Nov 10, 2023

@rv0lt Did you also revert the manual change you had done for the package?

@rv0lt
Copy link
Member Author

rv0lt commented Nov 10, 2023

@rv0lt Did you also revert the manual change you had done for the package?

Yes, I undo the change (I only modified one line of packages.json) and then run npm update postcss --save

@i-oden
Copy link
Member

i-oden commented Nov 10, 2023

@rv0lt Did you also revert the manual change you had done for the package?

Yes, I undo the change (I only modified one line of packages.json) and then run npm update postcss --save

Just to double check: npm update <package> --save warns you if there are other package dependencies that break with the update?

@rv0lt
Copy link
Member Author

rv0lt commented Nov 10, 2023

@rv0lt Did you also revert the manual change you had done for the package?

Yes, I undo the change (I only modified one line of packages.json) and then run npm update postcss --save

Just to double check: npm update <package> --save warns you if there are other package dependencies that break with the update?

Nop. And also, In this case npm audit fix (without force) will only safely update postcss. That is why I made the distinction with it and the other modules

@i-oden
Copy link
Member

i-oden commented Nov 10, 2023

@rv0lt Did you also revert the manual change you had done for the package?

Yes, I undo the change (I only modified one line of packages.json) and then run npm update postcss --save

Just to double check: npm update <package> --save warns you if there are other package dependencies that break with the update?

Nop. And also, In this case npm audit fix (without force) will only safely update postcss. That is why I made the distinction with it and the other modules

I'm not sure I follow. You split up the change with the other modules as you mentioned before but just to check here -- are we certain that this upgrade of the postcss does not break any other packages?

@rv0lt
Copy link
Member Author

rv0lt commented Nov 10, 2023

I'm not sure I follow. You split up the change with the other modules as you mentioned before but just to check here -- are we certain that this upgrade of the postcss does not break any other packages?

When running npm audit fix the only package that is updated is the PostCSS. The rest (tough-cookie, semver) are identified as vulnerable but shout the warning that can cause breaks.

That is why I thought it was better to split. This one can be patched, whereas the other require more careful examination.

Example of output with the dry-run flag to only show the changes

image

@i-oden
Copy link
Member

i-oden commented Nov 10, 2023

I'm not sure I follow. You split up the change with the other modules as you mentioned before but just to check here -- are we certain that this upgrade of the postcss does not break any other packages?

When running npm audit fix the only package that is updated is the PostCSS. The rest (tough-cookie, semver) are identified as vulnerable but shout the warning that can cause breaks.

That is why I thought it was better to split. This one can be patched, whereas the other require more careful examination.

Example of output with the dry-run flag to only show the changes
image

Aah ok, I follow!

Copy link
Member

@i-oden i-oden left a comment

Choose a reason for hiding this comment

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

Seems to work as it should, and changelog didn't include any strange changes.

@rv0lt rv0lt merged commit 9d337f6 into dev Nov 10, 2023
14 checks passed
@rv0lt rv0lt deleted the DDS-1812-Fix-CVE-2023-44270 branch November 10, 2023 14:04
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.

2 participants