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 highlight.js from 9.18.5 to 10.7.3 to solve security concerns #4045

Merged
merged 1 commit into from
May 18, 2023

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented May 16, 2023

Description

The highlight.js package is a syntax highlighter for web and node.js. The minimum version required to solve this security concern for highlight.js is 10.4.1. Currently, osd-ui-framework is using 9.18.5.

ubuntu@ip-172-31-55-237:~/work/OpenSearch-Dashboards$ yarn why highlight.js
yarn why v1.22.19
[1/4] Why do we have the module "highlight.js"...?
[2/4] Initialising dependency graph...
warning Resolution field "typescript@4.0.2" is incompatible with requested version "typescript@~4.8.4"
warning Resolution field "shelljs@0.8.5" is incompatible with requested version "shelljs@^0.6.0"
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "highlight.js@9.18.5"
info Has been hoisted to "highlight.js"
info Reasons this module exists
   - "workspace-aggregator-f6077111-5d3a-4964-a223-4b30b595099f" depends on it
   - Hoisted from "_project_#@elastic#eui#highlight.js"
   - Hoisted from "_project_#@osd#ui-framework#highlight.js"
=> Found "lowlight#highlight.js@10.4.1"
info This module exists because "_project_#@elastic#eui#remark-highlight.js#lowlight" depends on it.
Done in 1.23s.

In OpenSearch Dashboards, highlight.js is directly called below:

import PropTypes from 'prop-types';
import React, { Component } from 'react';

import classNames from 'classnames';
import hljs from 'highlight.js';

export class GuideCodeViewer extends Component {
  constructor(props) {
    super(props);
  }

  componentDidUpdate() {
    if (this.refs.html) {
      hljs.highlightBlock(this.refs.html);
    }

    if (this.refs.javascript) {
      hljs.highlightBlock(this.refs.javascript);
    }
  }

  renderSection(type, code) {
    const typeToCodeClassMap = {
      JavaScript: 'javascript',
      HTML: 'html',
    };

    const codeClass = typeToCodeClassMap[type];

    if (code) {
      return (
        <div className="guideCodeViewer__section" key={type}>
          <div className="guideCodeViewer__title">{type}</div>
          <pre className="guideCodeViewer__content">
            <code ref={codeClass} className={codeClass}>
              {code}
            </code>
          </pre>
        </div>
      );
    }
  }

  render() {
    const classes = classNames('guideCodeViewer', {
      'is-code-viewer-open': this.props.isOpen,
    });

    const codeSections = this.props.source.map((sourceObject) =>
      this.renderSection(sourceObject.type, sourceObject.code)
    );

    return (
      <div className={classes}>
        <div className="guideCodeViewer__header">{this.props.title}</div>

        <div className="guideCodeViewer__closeButton fa fa-times" onClick={this.props.onClose} />

        {codeSections}
      </div>
    );
  }
}

GuideCodeViewer.propTypes = {
  isOpen: PropTypes.bool,
  onClose: PropTypes.func,
  title: PropTypes.string,
  source: PropTypes.array,
};

In the above code snippet, highlight.js is used to highlight code blocks in a React component named GuideCodeViewer. The highlightBlock method takes a DOM node and highlights the text within it according to the language of the code .

Since there are over 300+ commits between highlight 9.18.5 and 10.4.1. It is hard to determine whether there are breaking changes. Let’s instead compare with highlightBlockfunction definitions from these two versions 10.4.1 https://github.com/highlightjs/highlight.js/blob/e96b915af70d1c3f014b732c10e7cd077f22b9c3/src/highlight.js#L676 and 9.18.5 https://github.com/highlightjs/highlight.js/blob/f54e96c24325f077a027bb950dcd9f8f3ef48b16/src/highlight.js#L907.

  • In version 10.4.1, two events, before:highlightBlock and after:highlightBlock are fired. From the above code snippet in OpenSearch Dashboards, we do not appear to use these events, so this change is unlikely to affect us.
  • There is a new property relavance added to the result and second_best object in the 10.4.1 version. The old property re is marked as deprecated and is planned to be removed in version 11.0. We don’t use these properties, so this change should not affect us.
  • The change from var to let for the node variable is a part of the modernization of the JavaScript codebase and doesn't affect the functionality.

Based on this analysis, there doesn't appear to be any breaking changes in the highlightBlock function between the two versions. So the fix could just be bump the package to 10.4.1.

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #4045 (24cf8bd) into 1.x (5c609bf) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              1.x    #4045      +/-   ##
==========================================
- Coverage   67.50%   67.50%   -0.01%     
==========================================
  Files        3044     3044              
  Lines       58692    58692              
  Branches     8902     8902              
==========================================
- Hits        39621    39619       -2     
- Misses      16923    16925       +2     
  Partials     2148     2148              
Flag Coverage Δ
Linux 67.45% <ø> (ø)
Windows 67.45% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@ananzh ananzh changed the title Bump highlight.js from 9.18.5 to 10.4.1 to solve security concerns Bump highlight.js from 9.18.5 to 10.7.3 to solve security concerns May 18, 2023
@ananzh ananzh changed the title Bump highlight.js from 9.18.5 to 10.7.3 to solve security concerns Bump highlight.js from 9.18.5 to 10.7.3 to solve security concerns May 18, 2023
@ananzh ananzh mentioned this pull request May 18, 2023
@seanneumann seanneumann merged commit d71c377 into opensearch-project:1.x May 18, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-1.x 1.x
# Navigate to the new working tree
pushd ../.worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-4045-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d71c377ad8ae4d48579dff32d63de5e9679479b1
# Push it to GitHub
git push --set-upstream origin backport/backport-4045-to-1.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-4045-to-1.x.

opensearch-trigger-bot bot pushed a commit that referenced this pull request May 18, 2023
…4045)

Signed-off-by: ananzh <ananzh@amazon.com>
(cherry picked from commit d71c377)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ananzh pushed a commit that referenced this pull request May 18, 2023
…4045) (#4062)

(cherry picked from commit d71c377)

Signed-off-by: ananzh <ananzh@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.3 security fix Security fix generated by Mend v1.3.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants