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

Bump node-sass to address LibSass vulnerabilities #501

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

tmarkley
Copy link
Contributor

Description

There are a few known security vulnerabilities related to the version of node-sass used in the repo. Both node-sass and LibSass are deprecated, but replacing node-sass with dart-sass fails because EUI does not follow the standard Sass spec. This results in a SassError: Top-level selectors may not contain the parent selector "&". Resolving this problem will have to be done in the long-term, but for now there are branches of node-sass that exist with a newer version of LibSass that does not contain any known security vulnerabilities. Unfortunately, these changes don't exist in any of the main releases, so we must use a specific branch (v5).

Details are on the main Sass website: https://sass-lang.com/blog/libsass-is-deprecated

Node-sass Issue for LibSass 3.6: sass/node-sass#2685

node-sass v5 branch: https://github.com/sass/node-sass/tree/v5

Signed-off-by: Tommy Markley [email protected]

Issues Resolved

N/A

Testing

Screen Shot 2021-06-18 at 3 02 42 PM

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

@tmarkley tmarkley added dependencies Pull requests that update a dependency file v1.0.0 labels Jun 18, 2021
@tmarkley tmarkley requested review from kavilla, boktorbb and ananzh June 18, 2021 20:03
@tmarkley tmarkley self-assigned this Jun 18, 2021
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 4c795fb

ananzh
ananzh previously approved these changes Jun 18, 2021
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM. Nice to learn.

boktorbb
boktorbb previously approved these changes Jun 21, 2021
Copy link
Contributor

@boktorbb boktorbb left a comment

Choose a reason for hiding this comment

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

LGTM

There are a few known security vulnerabilities related to the version
of node-sass used in the repo. Both node-sass and LibSass are
deprecated, but replacing node-sass with dart-sass fails because EUI
does not follow the standard Sass spec. This results in a SassError:
`Top-level selectors may not contain the parent selector "&"`. Resolving
this problem will have to be done in the long-term, but for now there are
branches of node-sass that exist with a newer version of LibSass that
does not contain any known security vulnerabilities. Unfortunately, these
changes don't exist in any of the main releases, so we must use a specific
branch (v5).

Details are on the main Sass website:
https://sass-lang.com/blog/libsass-is-deprecated

Signed-off-by: Tommy Markley <[email protected]>
@tmarkley tmarkley dismissed stale reviews from boktorbb and ananzh via 24b02f9 June 22, 2021 18:24
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 24b02f9

@tmarkley
Copy link
Contributor Author

All tests are passing after rebasing.

Screen Shot 2021-06-22 at 4 26 05 PM

@tmarkley tmarkley requested a review from ananzh June 22, 2021 21:26
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Smoke tested it, LGTM. We have an issue open for the long term fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file v1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants