chore: backport fork changes to upstream

[jlp-craigmorten]

Issue

Fixes #5

Relates to #6

Details

This cherry picks amended commits from the https://github.com/jlp-craigmorten/snyker fork of this repo to contribute changes back, namely:

• feat: upgrade dependencies to latest versions
• feat: bump to Node 20 and NPM >=9
• fix: correctly handle ignores even if limited to administrators
• chore: eslint v9 upgrade
• fix: need to force install to workaround incorrect peer deps on npm

(as included in the changelog)

This change squashes some patches I have on my fork into a single change against a new v5 version (hence why this change has 5.0.0 whereas my fork is currently at 5.0.3). Open to suggestion on whether you want to keep the version bump out of this change so you can implement yourselves, or change it as you better suited if disagree with the major.

Given the change in Node and NPM version it feels sensible to insist on this being a breaking change.

Also feel free to ignore or edit this change and/or recreate with the pieces you like.

[asos-danielc]

Nice one thanks @jlp-craigmorten for the re-contribution back!

The changes all look great, however, when running this locally as a test, I noticed that the package-lockfile.json is stripping the "integrity" properties out e.g.

Do you have any ideas on this?

[jlp-craigmorten]

Hey hey @asos-danielc!

The changes all look great, however, when running this locally as a test, I noticed that the package-lockfile.json is stripping the "integrity" properties out

Do you have any ideas on this?

It appears this might already be a behaviour in Snyker, see the shaPatch function in the current code https://github.com/ASOS/snyker/blame/main/src/index.js#L126.

Unfortunately this repo doesn't have the commit history (not sure if the internal repo does, or if it is now lost forever on an old and deleted asos-craigmorten repo...), so hard to recall exactly what this is for beyond the comment in the code. Vaguely recall the intent being that all sha1 hashes should be forced to update to sha256 hashes, but from your screenshot it looks like it's just binning it off completely...

I believe the intended behaviour was:

1. To strip out old sha1 hashes
2. Run an npm install --force
3. New sha256 hashes added and we're all good.

It seems that step (3) isn't happening for you... but that is very potentially a separate issue unless it relates to this change in how we now require the latest npm version etc. (and it worked on older npm versions).

Issues such as npm/cli#4263 indicate there might be an edge-case that I missed when this shaPatch logic was put in originally where integrity hashes aren't added back if you install from cache... perhaps worth trying a npm cache clear --force first and then running this version to see if it's better?

[asos-danielc]

Hey hey @asos-danielc!

The changes all look great, however, when running this locally as a test, I noticed that the package-lockfile.json is stripping the "integrity" properties out
Do you have any ideas on this?

It appears this might already be a behaviour in Snyker, see the shaPatch function in the current code https://github.com/ASOS/snyker/blame/main/src/index.js#L126.

Unfortunately this repo doesn't have the commit history (not sure if the internal repo does, or if it is now lost forever on an old and deleted asos-craigmorten repo...), so hard to recall exactly what this is for beyond the comment in the code. Vaguely recall the intent being that all sha1 hashes should be forced to update to sha256 hashes, but from your screenshot it looks like it's just binning it off completely...

I believe the intended behaviour was:

1. To strip out old sha1 hashes
2. Run an npm install --force
3. New sha256 hashes added and we're all good.

It seems that step (3) isn't happening for you... but that is very potentially a separate issue unless it relates to this change in how we now require the latest npm version etc. (and it worked on older npm versions).

Issues such as npm/cli#4263 indicate there might be an edge-case that I missed when this shaPatch logic was put in originally where integrity hashes aren't added back if you install from cache... perhaps worth trying a npm cache clear --force first and then running this version to see if it's better?

Interesting... so with your projects and testing, are you seeing the integrity property there still. I have followed your steps and getting the same result (integrity removed)

[asos-danielc]

@jlp-craigmorten, looking to progress this and will add a follow up PR to add a flag to disable the integrity removal. However its worth noting that this yarn and npm installs result in a warning or error (yarn and npm respectively) due to a dependency mismatch... which has been resolved with eslint-plugin-import https://github.com/import-js/eslint-plugin-import/releases/tag/v2.31.0

Have an issue with the snyk key needing updating which I am looking at too.