Repository navigation
fix(ci): clear the npm-vendored advisories from the release toolchain - #284
Conversation
The release toolchain's audit carried thirteen exemptions, twelve of them
packages that the npm CLI vendors - brace-expansion, http-cache-semantics,
ip-address, postcss-selector-parser and undici - reached through
@semantic-release/npm. They were listed as unfixable, and they are not:
npm 11.21.1 vendors fixed copies of all five, and @semantic-release/npm
requires ^11.6.2, so a lockfile refresh resolves it.
npm 11.21.0 -> 11.21.1
brace-expansion 5.0.9 -> 5.0.12
http-cache-semantics 4.2.0 -> 4.3.0
ip-address 10.5.0 -> 10.7.2
postcss-selector-parser 7.1.4 -> 7.1.6
undici 6.28.0 -> 6.29.0
EXEMPT is pruned to braces 3.0.3, the one advisory left - the latest release,
so no bump reaches it - and that entry carries its own reachability argument.
The header stops claiming blanket unreachability: an entry whose input arrives
from outside this repo is a risk acceptance, and says so.
The audit also stops merely warning about a stale exemption and fails on one.
A stale entry that is reported without failing is invisible in a passing
build, which is how those twelve sat: the audit printed them and exited 0.
Verified: node release/audit.mjs exits 0 with one exempt advisory; restoring
the previous lockfile makes it exit 1 with the twelve unexempted; npm ci
--prefix release --ignore-scripts installs npm 11.21.1 cleanly; prettier is
clean on the touched files. The change is confined to release/, which is not
shipped and is not imported by the package.
There was a problem hiding this comment.
Reviewed the full diff: the audit-gate rewrite in release/audit.mjs and the npm 11.21.1 bump in release/package-lock.json. Verified on a checkout of this head: the gate exits 0 with the single exempted braces advisory, a stale EXEMPT entry fails it (exit 1), the removed exemptions were all vendored inside the locked npm, and the lockfile integrity matches the registry.
- .github/workflows/run-tests.yml:33 - the PR audit step reads the lockfile only (--package-lock-only), so manifest-to-lockfile drift in release/ is invisible to it; the npm ci --prefix release --ignore-scripts companion runs only in publish.yml:90 (push to main), not on PRs. A release/package.json edit without its lockfile would pass this gate and fail the publish run on main. Carry the companion into run-tests.yml next to the audit, as sasjs/adapter's build-unit-tests.yml:67 does.
1 finding above for review.
The audit runs with `npm audit --package-lock-only`, so it reads the lockfile and never opens release/package.json. Drift between the two is therefore invisible to a pull request: a manifest edit without its lockfile passes the gate and fails the publish run on main. The install companion ran only in the publish workflow, which fires on a push to main. `npm ci --prefix release --ignore-scripts` now runs next to the audit in .github/workflows/run-tests.yml, the pairing sasjs/adapter's build-unit-tests.yml already carries. Verified by mutating the manifest rather than by inspection: adding a dependency to release/package.json leaves `node release/audit.mjs` at exit 0 and makes the install fail with EUSAGE on lockfile drift, and restoring the manifest returns the install to exit 0.
|
Finding addressed in The mechanism reproduces exactly as you describe. Adding a dependency to
|
There was a problem hiding this comment.
Re-review at the new head. Scope: the delta since the previously reviewed head, plus a re-read of the full base...head diff. The delta is one step in .github/workflows/run-tests.yml - npm ci --prefix release --ignore-scripts next to the release-toolchain audit - which makes release/package.json-to-lockfile drift fail on a pull request instead of in the publish run on main, resolving the prior finding. Verified: both CI workflows for this head (Linux and Windows) completed success. The rest of the diff (release/audit.mjs gate rewrite with the aggregated stale-exemption failure, npm 11.21.1 bump in release/package-lock.json) is unchanged from the previously reviewed head, so the earlier checkout verification carries over: the gate exits 0 with the single exempted braces advisory, a stale EXEMPT entry exits 1, and the lockfile integrity matches the registry. No blocking issues.
## [4.0.1](v4.0.0...v4.0.1) (2026-10-16) ### Bug Fixes * **ci:** clear the npm-vendored advisories from the release toolchain ([#284](#284)) ([b244756](b244756))
|
🎉 This PR is included in version 4.0.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What
The release toolchain's audit in
release/carried thirteen exemptions. Twelve of them were packages the npm CLI vendors -brace-expansion,http-cache-semantics,ip-address,postcss-selector-parserandundici- reached through@semantic-release/npm. They were documented as unfixable, and they are not: npm 11.21.1 vendors fixed copies of all five, and@semantic-release/npmrequiresnpm: ^11.6.2, so a lockfile refresh resolves it.npmbrace-expansionhttp-cache-semanticsip-addresspostcss-selector-parserundiciTwelve exemptions go with it.
EXEMPTkeepsbraces3.0.3, which is the latest release, so no bump reaches it - and that entry now carries its own reachability argument. The header stops claiming that every exemption is unreachable: an entry whose input arrives from outside this repo is a risk acceptance rather than a non-issue, and it says so.The audit also stops merely warning about a stale exemption and fails on one. A stale entry that is reported without failing is invisible in a passing build, which is how those twelve sat: the audit printed them and exited 0.
Verification
node release/audit.mjs: 1 exempt advisory, exit 0npm ci --prefix release --ignore-scripts: exit 0, npm 11.21.1 installedrelease/, which is not shipped and is not imported by the package - nothing in the published artifact movesNotes
Same fix as sasjs/vscode-extension#447, which is where the review that prompted it lives.
release/audit.mjsis the same script in adapter, cli, sas-language, utils and vscode-extension, butEXEMPTdescribes the tree it sits in, so each repo carries its own list and each needs its own lockfile refresh.