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

Upgrade to lerna 5.5.4 #11738

Merged
merged 2 commits into from
Oct 11, 2022
Merged

Upgrade to lerna 5.5.4 #11738

merged 2 commits into from
Oct 11, 2022

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented Oct 5, 2022

What it does

Fixes #11737

This PR updates dev-dependency lerna, from 4.x to latest 5.x. In the process we get rid of several important security vulnerabilities, that we have carried for a long while (that affect the development environment only, not Theia-based products at runtime).

We also introduce a new dev-dependency, improved-yarn-audit, that complements yarn audit nicely. Its behaviour is more configurable and its output parse-able, which makes it easier to eventually integrate to CI.

Simple usage examples:
$> yarn run improved-yarn-audit
$> yarn run improved-yarn-audit --ignore-dev-deps

How to test

  • confirm the repo build locally without issues and that the example application seems to work fine
  • confirm that the previous high and severe security vulnerabilities are gone - there is a new one at level high, related to the axios dependency of lerna, used by nx (they have a couple of related issues on their project, to use a later axios, so it will be fixed soon I hope)
  • (optional - I have done the below myself): test publishing a 'next' and 'latest' release using a local registry. Rough instructions:
    note: be careful if you happen to have the production registry configured on your machine. Consider moving the corresponding .npmrc file temporarily to avoid pushing to real registry by mistake
    • $> npm add -g verdaccio
    • $> verdaccio # start verdaccio in another terminal
    • in main terminal, set local registry as temporary default
    • $> npm config set registry http://localhost:4873/
    • $> yarn config set registry http://localhost:4873/
    • $> npm adduser --registry http://localhost:4873/ # use bogus user/credentials. e.g.: test/test/test@test.io
    • simulate solid release from the PR commit - select "minor" when prompted
    • $> git clean -ffdx && yarn && yarn build:examples && yarn test:theia
    • $> npx lerna publish --registry http://localhost:4873 --exact --yes --no-push && yarn -s publish:check
    • verify that publishing worked without errors from "publish:check"
    • locally build a local Theia application, using the newly locally released Theia (e.g. 1.31.0)
    • verify that the local app builds and runs fine
    • simulate next release - cleanup any checked-out file left before proceeding
    • $> git clean -ffdx && yarn && yarn build:examples && yarn test:theia
    • $> npx lerna publish --registry http://localhost:4873 preminor --exact --canary --preid next --dist-tag next --no-git-reset --no-git-tag-version --no-push --yes && yarn -s publish:check
    • verify that publishing worked without errors from "publish:check"
    • locally build a local Theia application, using the newly locally released Theia (e.g. 1.32.0-next.0)
    • verify that the local app builds and runs fine
    • when all done, unset local registry (else entries related to it will be included in yarn.lock as you continue working on something else:
    • $> npm config delete registry http://localhost:4873/
    • $> yarn config delete registry http://localhost:4873/

Review checklist

Reminder for reviewers

@marcdumais-work marcdumais-work force-pushed the lerna-5.5.4 branch 2 times, most recently from 7906684 to 28b253f Compare October 5, 2022 20:19
@marcdumais-work marcdumais-work changed the title [WIP] Upgrade to lerna 5.5.4 Upgrade to lerna 5.5.4 Oct 6, 2022
@vince-fugnitto vince-fugnitto added the dependencies pull requests that update a dependency file label Oct 6, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

  • Build is successful, and both the example-electron and example-browser work
  • CI passes
  • Confirmed that vulnerabilities previously reported by yarn audit have been resolved (new one from axios as expected)
  • Confirmed that yarn run improved-yarn-audit works and reports the vulnerabilities in a nice format
  • Confirmed that yarn run improved-yarn-audit --ignore-dev-deps works and does not report devDependencies

@marcdumais-work marcdumais-work force-pushed the lerna-5.5.4 branch 2 times, most recently from 084e353 to 2ec7ef0 Compare October 11, 2022 16:07
@marcdumais-work
Copy link
Contributor Author

@vince-fugnitto as suggested during the dev-meeting, I added an entry to the migration guide. And since there was a conflict with yarn.lock I rebased on latest master. Can you have another quick look?

Fixes #11737

before update:
7 vulnerabilities found - Packages audited: 1946
Severity: 3 Moderate | 2 High | 2 Critical

after update:
2 vulnerabilities found - Packages audited: 2036
Severity: 1 Moderate | 1 High

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
"improved-yarn-audit" (license: MIT), complements plain "yarn audit",
making audits easier to integrate in CI pipelines. The output is short
and to-the-point, making it useful immediately.

    Simple usage examples:
    $> yarn run improved-yarn-audit
    $> yarn run improved-yarn-audit --ignore-dev-deps

Here's the currint output for the Theia repo (with this PR in):

$> yarn run improved-yarn-audit
Improved Yarn Audit - v3.0.0

Minimum severity level to report: low

Running yarn audit...

Found 2 vulnerabilities

Vulnerability Found:

  Severity: MODERATE
  Modules: jsdom
  URL: GHSA-f4c9-cqv8-9v98

Vulnerability Found:

  Severity: HIGH
  Modules: lerna>nx>axios
  URL: GHSA-cph5-m8f7-6c5x

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[security] upgrade lerna from 4.x to 5.x
2 participants