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

breaking: fix audit failures; node 10+ #2680

Closed
wants to merge 7 commits into from
Closed

breaking: fix audit failures; node 10+ #2680

wants to merge 7 commits into from

Conversation

AviVahl
Copy link
Contributor

@AviVahl AviVahl commented Jul 31, 2020

Description

  • breaking: drop Node 6 and 8. support >=10.
  • ci: drop node 8 from Travis. add node 14.
  • bumped specific dependencies to get upstream audit fixes (npm audit fix --force).
  • removed unmaintained tacks dependency (has audit issues) and replaced tests using it with simple directory population.
  • adjust require.requireActual in tests to jest.requireActual, as former was dropped.
  • adjust expected error code from registry in lerna-publish-error.test.

Motivation and Context

closes #2682
closes #2606
closes #2678
closes #2676
closes #2677
closes #2662
closes #2647
closes #2623
closes #2575
closes #2492
closes #2570
closes #2579
partially closes #2691 (only fixed audit failures; deprecations require additional upgrades, which would be easier to do once this is in)

How Has This Been Tested?

npm test and npm run integration on Node 14/Fedora 32. (letting CI test other Node versions)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

- bumped specific deps to get upstream audit fixes (npm audit fix --force).
- removed unmaintained "tacks" and replaced tests using it with simple dir population.
- dropped node 8 from travis. added 14.
- changed engines for all packages to >=10.
- adjust require.requireActual in tests to jest.requireAction, as they dropped the former.
- adjust expected error code from registry in lerna-publish-error.test
instead, use same test command and test on all 3 node versions on windows as well
@AviVahl
Copy link
Contributor Author

AviVahl commented Jul 31, 2020

I've hit jestjs/jest#10339 :)
I'll wait for a bugfix and upgrade jest once out.

EDIT: v26.2.2 fixed the issue.

via lockfile tricks (external project + npm-force-resolutions)
@AviVahl
Copy link
Contributor Author

AviVahl commented Jul 31, 2020

@evocateur this one fixes the audit failures for the repo itself (devDependencies) as well...

I've simplified the CI config by removing the windows specific flow (and just letting it be a bit slower). I can revert that, if needed, but I really think it's not worth the infra overhead. Consistency of flow between different envs is important. Simplicity of scripts and configuration is also something worth striving for.

patch release supposed to fix windows-specific regression.
regenerated lock file from scratch for the better deduping
@MikeActually
Copy link

looks like the Jest fix is out jestjs/jest#10346 v 26.2.2

@AviVahl
Copy link
Contributor Author

AviVahl commented Aug 4, 2020

@MikeActually and it's included in this PR.

@@ -72,8 +71,8 @@
"fs-extra": "^8.1.0",
"glob-parent": "^5.1.0",
"globby": "^9.2.0",
"jest": "^24.9.0",
"jest-circus": "^24.9.0",
"jest": "^26.2.2",

Choose a reason for hiding this comment

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

ahhh, I missed this in my look-through; my apologies. Looks like this is good to go, then, I'm thinking?

@AviVahl
Copy link
Contributor Author

AviVahl commented Aug 4, 2020

Yep. :) Awaiting someone with permissions to review/merge/release.

@AviVahl
Copy link
Contributor Author

AviVahl commented Aug 9, 2020

@evocateur if you find yourself without the time to handle this PR, would you be open to accepting me as an additional maintainer to the project?

@tomrav
Copy link

tomrav commented Aug 13, 2020

Any chance of this PR getting merged? we would really appreciate the security fixes downstream.

@stratospheres
Copy link

+1 to finalizing this PR. We'd love to get our CI pipeline happy again. :)

@techtor
Copy link

techtor commented Aug 19, 2020

Friendly +1 on this for the sake of our CI pipeline.

@powerchelle
Copy link

@evocateur howdy! Any chance we can see your approval on this PR to get it pushed through? Our team needs this soon, in order to stay on top of security fixes.

@MikeActually
Copy link

I don't think @evocateur has been active in some time looking at profile page, but Lerna has other members, as per https://github.com/orgs/lerna/people . Maybe one of them can help out @gigabo @hzoo @ndelangen @xtuc

to pick up latest upstream minor/patch versions
@barak007
Copy link

+1000

@MikeActually
Copy link

as per https://www.npmjs.com/policies/disputes , I have emailed the two contacts listed for @evocateur and @hzoo and CCed NPM . I've also linked the repo and asked to either facilitate maintenance or to identify a new maintainer for this project. If there is no response after 4 weeks, we can look to move to adopt the library.

I do not have much interest in owning this long term, but will help facilitate transitioning this project, if the owners do not respond.

@MikeActually
Copy link

I've received a response from NPM, but still working on setting expectations that if nothing is heard from in 4 weeks, there will be an effort to add additional owners.

I did also spot this - #1172 , and there have not been many tweets for Lerna since this call for maintainers - https://twitter.com/lernajs

What options exist to truly make this open source? Is there a better strategy that exists? This package is very widely used but may need to be forked or go away if nobody maintains it. Is there a way to find out if any high profile organizations leverage this package and if they could commit resources to maintaining this?

@MikeActually
Copy link

I've also opened issue #2703 to help track the maintainer problem this repo has. I would like to encourage some discussion around how this can be addressed long term in a way that values the security of the users of this module, both from the perspective of that security patches need to regularly occur and that ownership/maintenance can occur in a way that the users have a high confidence of the maintainers

@MikeActually
Copy link

while not ideal, @AviVahl , I might recommend that this PR be distributed into smaller pieces, if possible, so that it might be easier to merge. I think there are even some scenarios that are questionably changed. For example, there is an updated test that changes the expected response code from NPM, I felt this was a bit presumptive, and it would be good to document where NPM changed this contract. I think to make it easier for the existing maintainers to move these security changes forward, making PRs that are concise would be a great help

@AviVahl
Copy link
Contributor Author

AviVahl commented Aug 25, 2020

Not ideal and not going to happen (at least from my end). I'm not going to spend further time on work being ignored. I've got more important things to do.

Also, imo, this PR isn't this big. Lock file changes are what's causing the large diff numbers. Changes themselves are mostly repetitive.

I've already noted the test change in the description. master also fails on that same test, and was hoping one of the maintainers will clarify whether this change is fine.

@MikeActually
Copy link

@AviVahl - that's fine, I would lik eto remind you that you did volunteer to be a maintainer of the project at #2680 (comment) . My apologies for assuming that you would have participated beyond just getting this merged.

regarding the contents of the PR, while components of it are repetitious, it does include multiple issues, instead of focusing on a singular change, which can sometimes be perceived as problematic.

@bharel-lmi
Copy link

Hi, is there going to be any progress with this PR?

Lerna currently has dependencies no libraries with vulnerabilities that make it a problem to use in certain organizations.
Is there a plan forward? Thank you.

@adantoscano
Copy link

Let's follow up this PR!

@AviVahl AviVahl closed this Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment