search

ScilifelabDataCentre / dds_web

A cloud-based system for the delivery of data from SciLifeLab Facilities to their users (e.g. research group).
Other
7 stars 7 forks source link

Patch Updating Postcss to solve CVE vulnerabity #1489

Closed rv0lt closed 10 months ago

rv0lt commented 10 months ago

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.

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.

codecov[bot] commented 10 months ago

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
i-oden commented 10 months ago

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

rv0lt commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

@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 commented 10 months ago

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 commented 10 months ago

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!