Repository navigation
fix(package): require node >= 22 and stop installing the release tooling - #8
Conversation
The declared node floor permits end-of-life releases - node 14 and 16 reached EOL in 2023, 18 on 2025-04-30, and 20 on 2026-04-30. The oldest line still receiving security fixes is 22, which is also the version CI runs. Raise engines.node to >=22 so the declaration matches the runtimes this package is tested and supported on. Verification: the only package.json change is the engines.node value, and CI runs on node 22.
semantic-release was a devDependency, and its tree is where every advisory in this repo came from: micromatch -> braces, and the npm package that @semantic-release/npm bundles (ip-address, undici, brace-expansion, http-cache-semantics). None of them has a patched release, so the audit carried twelve standing exemptions and two overrides to work around them. Run it the way sasjs/cli does instead - npx fetches the pinned pair for the release run only, so none of it enters the lockfile. The tree drops from 469 lock entries to 13, the overrides go, and scripts/audit.mjs - whose whole purpose was to name the unfixable advisories - is deleted. `npm audit --audit-level=low` now passes with no exemptions, in CI and in publish. The advisories are unfixable and unreachable: the braces DoS needs an attacker-controlled brace pattern, and the patterns here come from the repo's own release config, not from the commits being matched. Not auditing the tool therefore loses nothing. Verification: - npm audit --audit-level=low: 0 vulnerabilities - npm run lint: clean - npm test: 15 pass - package-lock.json: 13 entries (prettier, typescript and their deps)
c81975e to
20066e2
Compare
sasjs-dev
Bot
changed the title
![sasjs-dev[bot]](https://avatars.githubusercontent.com/u/67858352?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the full base...main diff (6 files: engines >=22, release tooling removed from devDependencies, audit.mjs deleted, overrides removed, workflows switched to plain strict audits, README updated) plus hardening checks at the head: pinning (all exact), .npmrc (ignore-scripts + save-exact + engine-strict), lockfile present and regenerated, gitleaks native hook (pinned @nogoo9/gitleaks binary, not neutered), CI audit gate (npm audit --audit-level=low across dev - strict, clean at the head, verified with npm audit --package-lock-only: 0 vulnerabilities), depcheck triage (clean; typescript is invoked via build:ts's tsc, a script-injection false positive), and secrets scan. CI (build, server) is green on the head. This is a coherent rework: with semantic-release out of the tree, the exemption-based audit script loses its reason and the plain audit is now clean. Findings:
-
.github/workflows/refresh.yml:23 - the refresh job still pins "node-version: 20" while package.json:72-73 now requires >=22. The two other workflows were bumped to 22 but this one was missed. Not cosmetic: .npmrc sets engine-strict=true and package.json is the manifest the job installs, so "npm ci" (refresh.yml:25, run without --ignore-scripts) fails immediately on node 20 with ENOTSUP (verified locally: engine-strict + a manifest engines conflict makes npm ci exit 1). The scheduled refresh will break on its next Monday run. Bump to 22.
-
.github/workflows/publish.yml:78-84 - "npx -p @semantic-release/exec@7.1.0 -p semantic-release@25.0.9 -- semantic-release": pinned top levels, and npx auto-installs without prompting in non-interactive CI (verified under npm 11). Same caveat as the org's other release workflows: the transitive tree is resolved fresh at release time, not locked, so a compromised transitive package executes with publish credentials. The old tree was at least lockfile-pinned (if advisory-ridden); the new one has no integrity lock at all. A dedicated lockfile or npm-shrinkwrap for the toolchain would pin the resolved tree. Advisory - the tradeoff is explicit in the comment, so this is a posture note, not a defect claim.
-
scripts/audit.mjs deletion: verified no remaining references - "npm run audit" script removed from package.json, no workflow calls it, README rewritten to describe the new posture. Dead code removal confirmed clean.
-
The overrides removal (ip-address/undici) is safe: those advisories lived inside the npm package @semantic-release/npm bundled, which is no longer installed anywhere (lockfile regenerated, full audit clean). Verified against the head lockfile audit.
2 findings above for review (refresh.yml node pin, release-toolchain lock posture).
.npmrc sets engine-strict=true, so the weekly refresh job's node 20 would now fail `npm ci` outright against the >=22 manifest - and the next scheduled run is Monday. Move it to 22, matching the other two jobs in the workflow.
|
Both addressed.
|
The release toolchain now lives in `release/` with its own package.json and lockfile, installed by the publish workflow with `npm ci --prefix release`, rather than being resolved from npx at release time. Locked, so the resolved versions and their integrity hashes are pinned - the gap raised in review, where npx pinned only the top levels and fetched the transitive tree fresh. The package's own `files` allowlist already keeps `release/` out of the tarball. Verification: every plugin in the release config resolves from release/node_modules.
The toolchain in release/ carries twelve advisories that cannot be fixed from this repo - the ranges cover the newest published version - and that code runs with publish credentials. Moving it out of the package's dependencies kept it out of the package's `npm audit`, which is right; it also made it invisible, which is not. release/audit.mjs reports it separately and by name: the twelve are exempt with their reasons, anything else fails the build, and an exemption that stops appearing is reported so it can be deleted rather than left to rot. It uses --package-lock-only, so it needs no install and runs on every PR as well as in the release. Verification: node release/audit.mjs reports 12 exempt and exits 0.
…p release] sas-language runs `prettier --check .`, which covers release/ - it is the only one of the five repos that checks that directory - and it rejected the new script. Formatted with that config (printWidth 100). The file is kept byte-identical in all five repos, so the same content goes to the four that do not check it. Otherwise the copies drift and a later fix lands in some of them only. Verification: `prettier --check .` clean, `node release/audit.mjs` still reports 12 exempt and exits 0, in all five.
There was a problem hiding this comment.
Re-review at the new head 45df3c8; the delta since the prior review at 20066e2 is the refresh.yml node bump (refresh.yml:23 now 22 - the prior finding, addressed) and the release-tooling lock tree (release/package.json + release/package-lock.json, npm ci --prefix release at publish.yml, semantic-release run from release/node_modules/.bin). The lock tree matches the release config's plugin set (@semantic-release/exec 7.1.0, semantic-release 25.0.9 - the release config uses commit-analyzer, release-notes-generator, exec and github, all resolved inside semantic-release's own tree or the locked tree, verified present in the lockfile). This also addresses the prior toolchain-lock posture note: the resolved tree and its integrity hashes are now pinned rather than fetched ad hoc with publish credentials. CI (build, server) is green on the head. The delta since the prior review adds release/audit.mjs - a scoped audit of the release toolchain with named, justified exemptions for the advisories that cannot be fixed from this repo (braces, brace-expansion, ip-address, undici inside the npm package @semantic-release/npm bundles) - sound. Hardening checks at the head all passed in the prior review and the delta does not touch them.
No blocking issues.
|
Thanks - nothing to action from this one. For the record, the only addition since the review before this one is |
|
🎉 This PR is included in version 0.3.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
What
Three changes, all about what this repo installs and runs.
Engines.
engines.nodewas>=20. Node 20 reached EOL on 2026-04-30 (18 on 2025-04-30); the oldest line still receiving security fixes is 22, which is also what CI runs. Raised to>=22..npmrcsetsengine-strict=true, so this is enforced on install rather than warned about.The refresh job. It pinned node 20 explicitly, which would now fail
npm ciagainst the raised floor. Moved to 22. The next scheduled run was Monday, so this would have gone red unattended.Release tooling.
semantic-releaseand@semantic-release/execare no longer devDependencies, and the toolchain no longer resolves fromnpxat release time either. It lives inrelease/with its own package.json and lockfile, installed by the publish workflow withnpm ci --prefix release. That pins the resolved versions and their integrity hashes rather than fetching the transitive tree fresh on every release - a tree that runs with publish credentials.With the toolchain out of this repo's dependency graph, the standing workarounds go too:
scripts/audit.mjsdeleted (its only job was naming the unfixable advisories)overridesforip-address/undicideletednpm audit --audit-level=low, which passes with zero exemptionsThe toolchain's own advisories, in their own scope. Keeping that tree out of the package's audit is correct - it is not shipped - but it must not become invisible either: it runs in CI with publish credentials.
release/audit.mjsaudits it separately and by name. Twelve advisories are exempt, each with the reason it cannot be fixed from this repo (the ranges cover the newest published version) and the reason it is unreachable here (thebracesdenial of service needs an attacker-controlled brace pattern, and the patterns come from this repo's own release config, not from the commits being matched). Anything else fails the build, and an exemption that stops appearing is reported so it can be deleted rather than left to rot. It uses--package-lock-only, so it needs no install and gates every PR as well as the release.Verification
package-lock.json: 469 -> 13 entries (prettier, typescript and their deps);release/package-lock.json: 453npm audit --audit-level=low: 0 vulnerabilitiesnode release/audit.mjs: 12 exempt, exit 0npm run lint: cleannpm test: 15 passrelease/node_modulesNotes
Running the tool from
release/instead ofnpxdoes not change what the tool does - it still executes at release time with publish credentials. What it changes is that the resolved versions and integrity hashes are pinned, and that the package's own audit is no longer the only place the risk could have shown up.