Fix npm ci failing on all CI jobs due to js-yaml override and remove deprecated codecov - #997
Conversation
|
OX Security reviewed this pull request — nothing to fix.
Branch |
npm ci failing on all CI jobs due to js-yaml overridenpm ci failing on all CI jobs due to js-yaml override and remove deprecated codecov
npm ci failing on all CI jobs due to js-yaml override and remove deprecated codecovnpm ci, remove deprecated codecov, and pin the Windows runner to 2025
|
I dislike pinning Windows. They should be synonymous, but perhaps the docs are lying. If we are going to unpin Windows, then let's also unpin Ubuntu to 26.04.
npm v12 drops that option, so it is time to start testing npm v12. |
| matrix: | ||
| node-version: ['20.x', '24.x', 'lts/*'] | ||
| os: [macos-latest, ubuntu-latest, ubuntu-24.04-arm, windows-latest] | ||
| os: [macos-latest, ubuntu-latest, ubuntu-24.04-arm, windows-2025] |
There was a problem hiding this comment.
windows-latest, windows-2025, and windows-2025-vs2026 should be synonymous.
https://github.com/actions/runner-images#available-images
There was a problem hiding this comment.
Hi @cclauss, you are right, it made no difference. I will revert that change.
I opened this PR to fix the failing CI to unblock #996, but this has exposed multiple issues with the CI. Do you want to try and resolve them all in this PR, or just keep it to resolving the lock file issue, and raise another PR to resolve the Windows tests?
There was a problem hiding this comment.
Smaller PRs are easier to review and merge.
Incrementalism takes patience but is a building block of open source.
There was a problem hiding this comment.
Yep, but it means merging this PR with failing tests...
There was a problem hiding this comment.
I'll create an issue for the failing Windows tests
There was a problem hiding this comment.
npm ci, remove deprecated codecov, and pin the Windows runner to 2025
Problem
Every workflow that runs
npm cifails during install, before any test or lint step executes:Example failing job: https://github.com/mapbox/node-pre-gyp/actions/runs/34881631492/job/104102066931
This affects
.github/workflows/ci.ymland.github/workflows/release.yml, both of which usenpm ci. It reproduces on a clean checkout ofmasterwith no other changes applied.Cause
82ec643 ("Support private S3 buckets", #923) added an
overridesblock topackage.json, presumably to liftjs-yamlclear of the DoS advisories affecting older 3.x releases:An override range is not pinned the way a dependency is.
npm cire-resolves every override against the registry before installing and compares the result with the lockfile, so a caret range is a live input on every run.That is why this broke without any commit to point at. When the override was added in November 2025,
^3.14.2resolved to3.14.2and matched the lockfile, so CI was green.js-yaml@3.15.0was published in June 2026, and from that point the same range resolved to3.15.xwhile the lockfile still recorded3.14.2. The3.15.2named in the error appears nowhere inpackage-lock.json: it is the freshly resolved value of the override.Fix
Two changes, both in service of making
npm cireproducible again.Remove the
overridesblock, and resolvejs-yamlforward in the lockfile. With no override left, the version is pinned by the lockfile alone and there is nothing for npm to re-resolve against the registry on each install. A futurejs-yamlrelease cannot reintroduce the same failure.Remove
codecovand itsupload-coveragescript. This is required rather than opportunistic:codecovpinsjs-yamlto exactly3.14.1, so dropping the override alone would let npm nest that vulnerable copy straight back into the tree. It cannot be upgraded out of the way either.3.8.3carries the pin and is the newest release on thelatesttag, published in 2021 and since deprecated by its maintainers in favour of thecodecov/codecov-actionGitHub Action.Removing it costs nothing, because it is already dead. The
upload-coveragescript was invoked from.travis.ymland has not run since Travis was removed in #718 (June 2024); the GitHub Actions workflows that replaced Travis never picked it up, and the repository has no codecov badge, config file, orCODECOV_TOKEN. Local coverage vianpm run coverage(nyc) is untouched.Result
No
3.14.1copy remains, soGHSA-2883-xcg3-v3hh(>=3.0.0 <3.15.2) is resolved rather than merely worked around. The pre-existingnpm lsELSPROBLEMSwarning, caused by codecov's unsatisfiable pin, is also cleared. 17 packages leave the dependency tree.