From c2afd3f3c8e281df9899a218a6bb9672358cd543 Mon Sep 17 00:00:00 2001 From: Rob Gilbreath Date: Thu, 3 Sep 2026 09:50:10 -0800 Subject: [PATCH] ci: Node and npm from org variables on both legs (shared-actions v3.0.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflows pass vars.NODE_VERSION / vars.NPM_VERSION to validate-codebase, validate-build and deploy, and every Dockerfile takes them as build args: `FROM node:${NODE_VERSION}-…` and `npm i -g "npm@${NPM_VERSION}"` in each stage that runs `npm ci`. No version literal remains in this repo — .nvmrc is gone, and so is the top-level verify-node-toolchain step (it asserted the runner's system node/npm, which nothing uses). Why: CI has pinned npm 12.0.2 since shared-actions v2.2.0, but images were built on node:24-bookworm's bundled npm 11. The tested tree and the shipped image disagreed on the dimension that caused the allowScripts incident. One org variable now moves both. pncit/.github#23 --- .github/actions/README.md | 3 +- .github/actions/validate-codebase/action.yml | 97 ++++++++++--------- .../actions/verify-node-toolchain/action.yml | 71 -------------- .../actions/verify-version-bump/action.yml | 2 +- .github/workflows/validate.yml | 25 ++--- .nvmrc | 1 - 6 files changed, 61 insertions(+), 138 deletions(-) delete mode 100644 .github/actions/verify-node-toolchain/action.yml delete mode 100644 .nvmrc diff --git a/.github/actions/README.md b/.github/actions/README.md index 2ae15da..40e5681 100644 --- a/.github/actions/README.md +++ b/.github/actions/README.md @@ -2,7 +2,7 @@ These are **local mirrors** (vendored copies) of composite actions from the private [`pncit/shared-actions`](https://github.com/pncit/shared-actions) repo, -currently at `v2.4.0` (`dfb52a3`). +currently at `v3.0.0` (`8bc187e`). ## Why they're copied here @@ -29,5 +29,4 @@ point. | local action | upstream | |---|---| | `validate-codebase/action.yml` | `pncit/shared-actions/.github/actions/validate-codebase` | -| `verify-node-toolchain/action.yml` | `pncit/shared-actions/.github/actions/verify-node-toolchain` | | `verify-version-bump/action.yml` | `pncit/shared-actions/.github/actions/verify-version-bump` | diff --git a/.github/actions/validate-codebase/action.yml b/.github/actions/validate-codebase/action.yml index 9cc13f3..d62c0be 100644 --- a/.github/actions/validate-codebase/action.yml +++ b/.github/actions/validate-codebase/action.yml @@ -16,9 +16,10 @@ description: >- # matches what their workflow needs to check. # # Does its own checkout + setup-node so it's self-contained — invoke -# directly from a job's `steps:` with just `uses:` and the npm-token. +# directly from a job's `steps:` with the npm-token and the two toolchain +# versions (`vars.NODE_VERSION`, `vars.NPM_VERSION`). -# Vendored from pncit/shared-actions@v2.4.0 (dfb52a3) because this repo is PUBLIC +# Vendored from pncit/shared-actions@v3.0.0 (8bc187e) because this repo is PUBLIC # and shared-actions is PRIVATE — a public repo can't `uses:` a private # action. Byte-for-byte identical to upstream apart from this comment; # re-copy when upstream changes (see .github/actions/README.md). @@ -36,42 +37,50 @@ inputs: required: false default: 'https://registry.npmjs.org' - npm-version: + node-version: description: >- - Exact npm version to pin after setup-node, e.g. "12.0.2". `.nvmrc` - pins a Node *major*, so setup-node resolves to whatever 24.x is - newest on the day — which is how npm crossed the 11 -> 12 boundary - (and started blocking unapproved install scripts) with nothing - committed on either side. Pinning here makes that upgrade a - deliberate, reviewed edit to one default instead of a surprise. - Set to the empty string to opt out and take the bundled npm. - required: false - default: '12.0.2' - node-major-version: + Exact Node version to install with setup-node, e.g. "24.20.0". + Callers pass `vars.NODE_VERSION`. There is deliberately no default + and no `.nvmrc` fallback: the same value is handed to `deploy` / + `validate-build` as the `NODE_VERSION` build arg, so the image is + built on the Node that tested it, and a bump is one org variable + instead of a per-repo sweep. Empty fails the job. + required: true + npm-version: description: >- - Optional. Required Node major, e.g. "24" — typically - vars.NODE_MAJOR_VERSION. When this and npm-major-version are both - set, verify-node-toolchain runs *after* setup-node, so it measures - the toolchain that actually runs `npm ci`. Callers that invoke - verify-node-toolchain as a top-level step are measuring the - runner's system Node/npm, which is not what this action uses. - required: false - default: '' - npm-major-version: - description: 'Optional. Required npm major, e.g. "12". See node-major-version.' - required: false - default: '' + Exact npm version to install after setup-node, e.g. "12.0.2". + Callers pass `vars.NPM_VERSION`; deploy / validate-build receive the + same value as the `NPM_VERSION` build arg. `.nvmrc`-style Node + majors let the bundled npm cross 11 -> 12 (and start blocking + unapproved install scripts) with nothing committed — pinning here + and in the Dockerfile from one variable is what stops that. Empty + fails the job. + required: true runs: using: composite steps: + # `required: true` doesn't catch an unset org variable — it arrives as + # "". Fail here with a message that names the variable rather than + # letting setup-node resolve "" to a surprise. + - name: Require toolchain versions + shell: bash + env: + NODE_VERSION: ${{ inputs.node-version }} + NPM_VERSION: ${{ inputs.npm-version }} + run: | + ok=true + [[ -n "$NODE_VERSION" ]] || { echo "::error::node-version is empty — set the org variable NODE_VERSION (or a repo override) and pass vars.NODE_VERSION."; ok=false; } + [[ -n "$NPM_VERSION" ]] || { echo "::error::npm-version is empty — set the org variable NPM_VERSION (or a repo override) and pass vars.NPM_VERSION."; ok=false; } + $ok + - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Setup Node uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version-file: .nvmrc + node-version: ${{ inputs.node-version }} # The GitHub cache service only pays off on a fresh VM. On the # self-hosted fleet ~/.npm persists between jobs, so `npm ci` is # already warm — while `cache: npm` cost every job with a changed @@ -89,11 +98,9 @@ runs: # v4-era flag and is no longer needed. Specifying it produced an # "Unexpected input" warning during the first real deploy run. - # Pin npm before anything reads it. setup-node gives us whatever npm - # its Node bundles; this makes the version an explicit, reviewable - # input instead of a moving target. + # setup-node gives us whatever npm its Node bundles; replace it before + # anything reads it. - name: Pin npm - if: inputs.npm-version != '' shell: bash run: | npm i -g "npm@${NPM_VERSION}" @@ -101,32 +108,28 @@ runs: env: NPM_VERSION: ${{ inputs.npm-version }} - # Runs here, after Setup Node and the pin, so it asserts against the - # toolchain `npm ci` will actually use. - # Inlined copy of verify-node-toolchain (keep the two in step). A nested - # `uses: pncit/shared-actions/...` can't be SHA-pinned to a release that - # doesn't exist yet, and it breaks the byte-for-byte vendored copies in - # the public repos (a nested private ref is resolved at prepare time even - # when the step's `if:` is false). + # Assert the toolchain `npm ci` is about to use is the one that was + # asked for — a tool-cache miss on a self-hosted runner can leave + # setup-node falling back to the system Node. Prefix match so a + # less-specific variable value ("24") still works if anyone sets one. - name: Verify Node toolchain - if: inputs.node-major-version != '' && inputs.npm-major-version != '' shell: bash env: - NODE_MAJOR_VERSION: ${{ inputs.node-major-version }} - NPM_MAJOR_VERSION: ${{ inputs.npm-major-version }} + WANT_NODE: ${{ inputs.node-version }} + WANT_NPM: ${{ inputs.npm-version }} run: | NODE_VERSION=$(node --version) NPM_VERSION=$(npm --version) echo "node: $NODE_VERSION ($(which node))" echo "npm: $NPM_VERSION ($(which npm))" - if [[ ! "$NODE_VERSION" =~ ^v${NODE_MAJOR_VERSION}\. ]]; then - echo "::error::Expected Node ${NODE_MAJOR_VERSION}.x, got $NODE_VERSION." - echo "::error::The runner is falling back to system Node — install Node ${NODE_MAJOR_VERSION} on the runner or fix actions/setup-node." + if [[ "$NODE_VERSION" != "v${WANT_NODE}" && "$NODE_VERSION" != "v${WANT_NODE}".* ]]; then + echo "::error::Expected Node ${WANT_NODE}, got $NODE_VERSION." + echo "::error::The runner is falling back to system Node — check actions/setup-node's download/tool-cache on this runner." exit 1 fi - if [[ ! "$NPM_VERSION" =~ ^${NPM_MAJOR_VERSION}\. ]]; then - echo "::error::Expected npm ${NPM_MAJOR_VERSION}.x, got $NPM_VERSION." - echo "::error::npm version mismatch resolves nested optional deps differently and breaks npm ci against the committed lockfile." + if [[ "$NPM_VERSION" != "${WANT_NPM}" && "$NPM_VERSION" != "${WANT_NPM}".* ]]; then + echo "::error::Expected npm ${WANT_NPM}, got $NPM_VERSION." + echo "::error::The npm pin did not take; npm version mismatch resolves nested optional deps differently and breaks npm ci against the committed lockfile." exit 1 fi @@ -155,7 +158,7 @@ runs: run: | if grep -q 'allowScripts' "${RUNNER_TEMP}/npm-ci.log"; then grep 'allowScripts' "${RUNNER_TEMP}/npm-ci.log" >&2 - echo "::warning::npm flagged or skipped one or more install scripts. If any is a native module, it did not build and the install still reported success. Approve with: npx npm@${NPM_VERSION:-12} install-scripts approve , then commit the package.json allowScripts change." + echo "::warning::npm flagged or skipped one or more install scripts. If any is a native module, it did not build and the install still reported success. Approve with: npx npm@${NPM_VERSION} install-scripts approve , then commit the package.json allowScripts change." fi env: NPM_VERSION: ${{ inputs.npm-version }} diff --git a/.github/actions/verify-node-toolchain/action.yml b/.github/actions/verify-node-toolchain/action.yml deleted file mode 100644 index 84079b0..0000000 --- a/.github/actions/verify-node-toolchain/action.yml +++ /dev/null @@ -1,71 +0,0 @@ -name: 'Verify Node toolchain' -description: >- - Fail-fast check that `node` and `npm` on the runner match the major - versions the repo expects. Catches the case where actions/setup-node - silently falls back to whatever's installed on a self-hosted runner - (e.g. the tool cache miss path), which would otherwise let a stale - Node version reach `npm ci` and resolve nested optional deps - differently from the committed lockfile. - -# Standalone use: add to any workflow that calls actions/setup-node: -# -# - uses: actions/setup-node@v7 -# with: -# node-version-file: .nvmrc -# - uses: pncit/shared-actions/.github/actions/verify-node-toolchain@v2 -# with: -# node-major-version: ${{ vars.NODE_MAJOR_VERSION }} -# npm-major-version: ${{ vars.NPM_MAJOR_VERSION }} -# -# `validate-codebase` calls this internally, but only when it is given -# both node-major-version and npm-major-version; it runs there *after* -# that action's own setup-node, so it measures the toolchain that will -# actually run `npm ci`. -# -# Invoking it as a top-level workflow step instead measures the runner's -# *system* node/npm, which is not what a composite doing its own -# setup-node will use. That gap is why an npm 11 -> 12 upgrade slipped -# past this gate: it reported npm 11.11.0 from /usr/bin/npm while -# `npm ci` ran under npm 12 from the tool cache. Prefer passing the -# versions through to `validate-codebase`. - -# Vendored from pncit/shared-actions@v2.1.0 (b164c08) because this repo is PUBLIC -# and shared-actions is PRIVATE — a public repo can't `uses:` a private -# action. Byte-for-byte identical to upstream apart from this comment; -# re-copy when upstream changes (see .github/actions/README.md). - -inputs: - node-major-version: - description: 'Required Node major version, e.g. "24". Typically sourced from vars.NODE_MAJOR_VERSION.' - required: true - npm-major-version: - description: 'Required npm major version, e.g. "11". Typically sourced from vars.NPM_MAJOR_VERSION.' - required: true - -runs: - using: composite - steps: - - name: Verify Node and npm match expected major versions - shell: bash - env: - NODE_MAJOR_VERSION: ${{ inputs.node-major-version }} - NPM_MAJOR_VERSION: ${{ inputs.npm-major-version }} - run: | - if [[ -z "$NODE_MAJOR_VERSION" || -z "$NPM_MAJOR_VERSION" ]]; then - echo "::error::node-major-version and npm-major-version inputs must both be set." - exit 1 - fi - NODE_VERSION=$(node --version) - NPM_VERSION=$(npm --version) - echo "node: $NODE_VERSION ($(which node))" - echo "npm: $NPM_VERSION ($(which npm))" - if [[ ! "$NODE_VERSION" =~ ^v${NODE_MAJOR_VERSION}\. ]]; then - echo "::error::Expected Node ${NODE_MAJOR_VERSION}.x, got $NODE_VERSION." - echo "::error::The runner is falling back to system Node — install Node ${NODE_MAJOR_VERSION} on the runner or fix actions/setup-node." - exit 1 - fi - if [[ ! "$NPM_VERSION" =~ ^${NPM_MAJOR_VERSION}\. ]]; then - echo "::error::Expected npm ${NPM_MAJOR_VERSION}.x, got $NPM_VERSION." - echo "::error::npm version mismatch resolves nested optional deps differently and breaks npm ci against the committed lockfile." - exit 1 - fi diff --git a/.github/actions/verify-version-bump/action.yml b/.github/actions/verify-version-bump/action.yml index 346858b..4e14ea6 100644 --- a/.github/actions/verify-version-bump/action.yml +++ b/.github/actions/verify-version-bump/action.yml @@ -36,7 +36,7 @@ description: >- # Does its own checkout (fetch-depth 2 plus a fetch of the reference), so it # can run first in a job; a later validate-codebase will check out again. -# Vendored from pncit/shared-actions@v2.4.0 (dfb52a3) because this repo is PUBLIC +# Vendored from pncit/shared-actions@v3.0.0 (8bc187e) because this repo is PUBLIC # and shared-actions is PRIVATE — a public repo can't `uses:` a private # action. Byte-for-byte identical to upstream apart from this comment; # re-copy when upstream changes (see .github/actions/README.md). diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 2d4fdb8..6cf79b9 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -49,21 +49,6 @@ jobs: with: fetch-depth: 2 - # Install the repo's Node (.nvmrc) before the toolchain check: the hosted - # image's system Node is whatever Ubuntu ships, not necessarily ours. - # validate-codebase below runs setup-node again; that's a cache hit. - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 - with: - node-version-file: .nvmrc - - # Env check: fail fast if the Node/npm majors don't match what the repo - # expects (NODE_MAJOR_VERSION / NPM_MAJOR_VERSION are pncit org-level - # Actions variables). Also catches .nvmrc drifting from the org standard. - - uses: ./.github/actions/verify-node-toolchain - with: - node-major-version: ${{ vars.NODE_MAJOR_VERSION }} - npm-major-version: ${{ vars.NPM_MAJOR_VERSION }} - # Version-bump gate: @pncit/node-quickbooks is a published library, so # every PR into master must bump package.json's version, or # publish-on-version-bump has nothing to publish after the squash. @@ -73,11 +58,19 @@ jobs: # HEAD^ and fail every unversioned merge; npm-publish.yml already skips # when there is nothing new to publish. - uses: ./.github/actions/verify-version-bump + with: + # .nvmrc went with shared-actions v3.0.0 (Node is vars.NODE_VERSION now); + # deleting it ships nothing, so it must not force a release either. + exempt-paths: | + .github/** + .nvmrc if: github.event_name == 'pull_request' # Codebase checks: npm ci + lint + typecheck + test. This action does its - # own checkout + setup-node (node-version-file: .nvmrc) and matches the + # own checkout + setup-node (vars.NODE_VERSION) and matches the # @pncit scope / npmjs registry by default. - uses: ./.github/actions/validate-codebase with: npm-token: ${{ secrets.NPM_TOKEN }} + node-version: ${{ vars.NODE_VERSION }} + npm-version: ${{ vars.NPM_VERSION }} diff --git a/.nvmrc b/.nvmrc deleted file mode 100644 index a45fd52..0000000 --- a/.nvmrc +++ /dev/null @@ -1 +0,0 @@ -24