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

Fixes #1562: fixes version of trim-newlines #1563

Merged
merged 1 commit into from Jul 2, 2021

Conversation

ivolzhevbt
Copy link
Contributor

Description

Hi maintainers! I'm Ivan and I work at Bison Trails. We're doing an internal hackathon this week.

Our team loves vscode-gitlens, but it got flagged during a recent security review. Our team voted to use our hackathon time to patch vulnerabilities in our favorite OSS tools and specifically to try to get vscode-gitlens approved for usage within our organization.

What's here:

  • version of trim-newlines dependency is forced to resolve to the safe, patched version of the package

Impact:

These changes should improve vscode-gitlens's security posture. We identified these vulnerabilities using Salus and the OSSF scorecard. The specific issues I am hoping to address here are:

  • Regular expression Denial of service described in this advisory

Thank you so much for your work maintaining vscode-gitlens, and thank you for taking the time to look at these proposed changes!

Checklist

  • I have followed the guidelines in the Contributing document
  • My changes follow the coding style of this project
  • My changes build without any errors or warnings - there is a warning because change forces newer version of trim-newlines package included through devDependencies dependencies. Newer version is properly patched, and backward compatible. I haven't found a way to replace direct dependency which would have updated trim-newlines transitive dependency.
  • My changes have been formatted and linted
  • My changes include any required corresponding changes to the documentation
  • My changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • My changes have a descriptive commit message with a short title, including a Fixes $XXX - or Closes #XXX - prefix to auto-close the issue that your PR addresses

trim-newlines is a transitive dependency which is present in a
dependency tree through imagemin-webp and node-sass.

Version of trim-newlines which is referenced by those pacakges has a
security advisory https://snyk.io/vuln/SNYK-JS-TRIMNEWLINES-1298042

Neither imagemin-webp nor node-sass have version which depend on patched
version of trim-newlines. And at least node-sass is not maintained any
longer.

Under current circumstances the only way to fix this is to force
trim-newlines version via "resolutions". As interface of the package is
backward compatible and does not break anything I believe it is a right
thing to do.
@ivolzhevbt ivolzhevbt changed the title Fixes 1562: fixes version of trim-newlines Fixes #1562: fixes version of trim-newlines Jun 29, 2021
@eamodio eamodio self-assigned this Jul 2, 2021
@eamodio eamodio added this to the Soon™ milestone Jul 2, 2021
@eamodio eamodio merged commit c5428ed into gitkraken:main Jul 2, 2021
@eamodio
Copy link
Member

eamodio commented Jul 2, 2021

Thank you for your contribution!

Thank you

@eamodio eamodio modified the milestones: Soon™, Shipped Jul 13, 2021
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.

None yet

2 participants