From fc0f1c61b54a79c6a354c8f9dbe821bc10a98893 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Mon, 25 May 2026 20:51:57 +0100 Subject: [PATCH] refactor!: add a new `pack` subaction to harden workflow (#23) --- README.md | 36 +++++++++++++-- SECURITY.md | 2 +- action.yml | 5 +- pack/action.yml | 104 ++++++++++++++++++++++++++++++++++++++++++ publish/action.yml | 36 +++------------ scripts/pack.ts | 83 +++++++++++++++++++++++++++++++++ scripts/pin-readme.ts | 2 +- scripts/publish.ts | 53 +++++++++------------ 8 files changed, 253 insertions(+), 68 deletions(-) create mode 100644 pack/action.yml create mode 100644 scripts/pack.ts diff --git a/README.md b/README.md index dd0fa98..560b20c 100644 --- a/README.md +++ b/README.md @@ -79,16 +79,32 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} # The chained dispatch from `release` lands here as a `workflow_dispatch` - # event on a `vX.Y.Z` tag ref. Manual recovery uses the same path + # event on a `vX.Y.Z` tag ref. The `pack` job installs deps, runs + # `pnpm pack` (or `npm pack`), and uploads the tarball as a workflow + # artifact. Lifecycle scripts (`prepack`, `prepare`, `postpack`) run + # here, in a job with `permissions: {}` and no `npm` environment. + # Manual recovery uses the same path. # (Run workflow -> pick a `v*` tag). + pack: + if: github.event_name == 'workflow_dispatch' && startsWith(github.ref, 'refs/tags/v') + runs-on: ubuntu-latest + concurrency: + group: pack-${{ github.ref }} + cancel-in-progress: false + permissions: {} + steps: + - uses: danielroe/uppt/pack@8da0597c0c549fdfd95a9868df95feb6d04dab26 # v0.4.0 + + # `publish` downloads the prebuilt tarball from the pack job's + # artifact and stages it for publish. publish: if: github.event_name == 'workflow_dispatch' && startsWith(github.ref, 'refs/tags/v') + needs: pack runs-on: ubuntu-latest concurrency: group: publish-${{ github.ref }} cancel-in-progress: false permissions: - contents: read # checkout the tag id-token: write # OIDC claim for npm trusted publisher environment: npm # must match the trusted-publisher entry on npmjs.com steps: @@ -125,15 +141,27 @@ When you merge a release PR, this subaction tags that commit, creates a GitHub R | `publish-workflow` | `release.yml` | Workflow filename to dispatch after tagging. Must declare `workflow_dispatch`. | | `checkout` | `true` | Set to `false` if the caller has already checked out `github.event.pull_request.merge_commit_sha`. | +### Packs a tarball (`danielroe/uppt/pack`) + +This subaction installs the package's dependencies, runs `pnpm pack` (if you have a `pnpm-lock.yaml`) or `npm pack`, and uploads each resulting `.tgz` as a workflow artifact for the `publish` job to consume. + +| Input | Default | Description | +| --- | --- | --- | +| `node-version` | `24` | Node version for the scripts. Needs `--experimental-strip-types` (Node 22.6+, 24+ recommended). Ignored when `install` is `false`. | +| `checkout` | `true` | Set to `false` if the caller has already checked out the tag ref. | +| `install` | `true` | Set to `false` to handle `actions/setup-node` and dependency installation yourself. Useful when you want a pinned package manager version, a cached `node_modules`, or a hardened install policy. When `false`, the caller must put `node`, `npm`, and any package manager on PATH before `uppt/pack` runs. | + ### Stages a publish (`danielroe/uppt/publish`) -This subaction runs `pnpm pack` (if you have a `pnpm-lock.yaml`) and then runs `npm stage publish` with OIDC authentication. The staged version then needs to be approved by a maintainer with 2FA on npmjs.com before it goes live. +This subaction downloads the tarball uploaded by `uppt/pack` in the same workflow run and runs `npm stage publish ./.tgz` with OIDC authentication. The staged version then needs to be approved by a maintainer with 2FA on npmjs.com before it goes live. + +> [!IMPORTANT] +> `prepublishOnly` is **not** invoked: `uppt/publish` publishes the prebuilt tarball with `--ignore-scripts`. Move any logic you previously had in `prepublishOnly` into `prepack` so it runs during `uppt/pack` and the output lands in the tarball. | Input | Default | Description | | --- | --- | --- | | `node-version` | `24` | Node version for the scripts and for `npm stage publish`. Needs `--experimental-strip-types` (Node 22.6+, 24+ recommended). | | `npm-access` | `public` | npm access level (`public` or `restricted`). | -| `checkout` | `true` | Set to `false` if the caller has already checked out the tag ref. | ## Prerequisites diff --git a/SECURITY.md b/SECURITY.md index 7aa3afa..428e9b5 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -33,7 +33,7 @@ Out of scope: These are deliberate trade-offs, documented here so they're not surprises: -- **`uppt/publish` runs the caller's lifecycle scripts in the same job that holds the npm OIDC token.** `pnpm install` and `npm ci` run with `--ignore-scripts`, but `pnpm pack` (used when a `pnpm-lock.yaml` is present) executes the package's own `prepack`/`prepare`/`prepublishOnly` scripts. A compromised script in the caller's own package will run with the npm OIDC token in scope. uppt does not sandbox lifecycle scripts. +- **Lifecycle scripts run during pack, not during publish.** `uppt/pack` installs deps and runs `pnpm pack` (or `npm pack`) in a job that the recommended workflow runs with `permissions: {}`. `pnpm install` and `npm ci` themselves use `--ignore-scripts`, but `pnpm pack` and `npm pack` execute the package's own `prepack`/`prepare`/`postpack` scripts (this is where most packages run their TypeScript or bundler build, so disabling them is not a viable default). uppt does not sandbox lifecycle scripts. ## Coordinated disclosure diff --git a/action.yml b/action.yml index 5b0cae1..80a2e8f 100644 --- a/action.yml +++ b/action.yml @@ -12,10 +12,11 @@ runs: - shell: bash run: | cat <<'EOF' >&2 - ::error::danielroe/uppt is split into three subactions. Pick one: + ::error::danielroe/uppt is split into four subactions. Pick one: - danielroe/uppt/pr@ (push to default branch) - danielroe/uppt/release@ (pull_request: closed, release/v* head ref) - - danielroe/uppt/publish@ (push or workflow_dispatch on a v* tag) + - danielroe/uppt/pack@ (workflow_dispatch on a v* tag; uploads tarball as release asset) + - danielroe/uppt/publish@ (workflow_dispatch on a v* tag; runs npm stage publish) See https://github.com/danielroe/uppt#getting-started for the full workflow. EOF exit 1 diff --git a/pack/action.yml b/pack/action.yml new file mode 100644 index 0000000..bc5c2e3 --- /dev/null +++ b/pack/action.yml @@ -0,0 +1,104 @@ +name: 'uppt/pack' +description: 'Pack the current tag into a tarball and upload it as a workflow artifact for the publish job to consume.' +author: 'Daniel Roe' + +branding: + icon: 'archive' + color: 'black' + +inputs: + node-version: + description: 'Node version for the scripts. Needs `--experimental-strip-types` support (22.6+, 24+ recommended).' + required: false + default: '24' + checkout: + description: 'Whether the action should run `actions/checkout` itself. Set to `false` if the caller has already checked out the tag ref.' + required: false + default: 'true' + install: + description: 'Whether the action should install the package''s dependencies itself. Set to `false` if the caller has already installed (e.g. with a pinned package manager version, a cached `node_modules`, or a hardened install policy). When `false`, the action will not run `actions/setup-node` either; the caller is responsible for putting `node`, `npm`, and any package manager on PATH before `uppt/pack` runs.' + required: false + default: 'true' + +runs: + using: composite + steps: + - name: Validate event + shell: bash + env: + EVENT_NAME: ${{ github.event_name }} + run: | + set -euo pipefail + case "$EVENT_NAME" in + push|workflow_dispatch) ;; + *) + echo "::error::danielroe/uppt/pack only supports 'push' and 'workflow_dispatch' events, got '$EVENT_NAME'." + exit 1 + ;; + esac + if [ "${GITHUB_REF:-}" = "${GITHUB_REF#refs/tags/v}" ]; then + echo "::error::danielroe/uppt/pack expected a 'refs/tags/v*' ref, got '${GITHUB_REF:-}'." + exit 1 + fi + + - name: Checkout + if: inputs.checkout == 'true' + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - name: Setup Node + if: inputs.install == 'true' + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: ${{ inputs.node-version }} + package-manager-cache: false + + - name: Install dependencies + if: inputs.install == 'true' + shell: bash + env: + COREPACK_ENABLE_STRICT: '1' + COREPACK_ENABLE_DOWNLOAD_PROMPT: '0' + run: | + set -euo pipefail + pm=$(node -e 'try { const p = require("./package.json").packageManager || ""; const m = p.match(/^(pnpm|yarn|npm)@/); process.stdout.write(m ? m[1] : "") } catch { process.stdout.write("") }') + if [ -z "$pm" ]; then + if [ -f pnpm-lock.yaml ]; then pm=pnpm + elif [ -f yarn.lock ]; then pm=yarn + elif [ -f package-lock.json ]; then pm=npm + else + echo "::error::danielroe/uppt/pack found no \`packageManager\` field in package.json and no lockfile (pnpm-lock.yaml, yarn.lock, or package-lock.json). Refusing to install from unpinned dependencies. Set \`packageManager\` and commit a lockfile, or pass \`install: false\` and install dependencies yourself before this step runs." + exit 1 + fi + fi + case "$pm" in + pnpm) + [ -f pnpm-lock.yaml ] || { echo "::error::\`packageManager\` declares pnpm but pnpm-lock.yaml is missing."; exit 1; } + corepack enable + pnpm install --frozen-lockfile --ignore-scripts + ;; + yarn) + [ -f yarn.lock ] || { echo "::error::\`packageManager\` declares yarn but yarn.lock is missing."; exit 1; } + corepack enable + yarn install --immutable --mode=skip-build || yarn install --frozen-lockfile --ignore-scripts + ;; + npm) + [ -f package-lock.json ] || { echo "::error::\`packageManager\` declares npm but package-lock.json is missing."; exit 1; } + npm ci --ignore-scripts + ;; + esac + + - name: Pack tarball(s) + shell: bash + env: + PACK_OUT_DIR: ${{ runner.temp }}/uppt-pack + run: node --experimental-strip-types ${{ github.action_path }}/../scripts/pack.ts + + - name: Upload tarball artifact + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: uppt-tarball + path: ${{ runner.temp }}/uppt-pack/*.tgz + if-no-files-found: error + retention-days: 1 diff --git a/publish/action.yml b/publish/action.yml index 190e4a7..3b233f1 100644 --- a/publish/action.yml +++ b/publish/action.yml @@ -1,5 +1,5 @@ name: 'uppt/publish' -description: 'Pack and stage-publish to npm using OIDC trusted publishing.' +description: 'Stage-publish a prebuilt tarball to npm using OIDC trusted publishing.' author: 'Daniel Roe' branding: @@ -15,10 +15,6 @@ inputs: description: 'npm access level (`public` or `restricted`).' required: false default: 'public' - checkout: - description: 'Whether the action should run `actions/checkout` itself. Set to `false` if the caller has already checked out the tag ref.' - required: false - default: 'true' runs: using: composite @@ -41,12 +37,6 @@ runs: exit 1 fi - - name: Checkout - if: inputs.checkout == 'true' - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - name: Setup Node uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: @@ -58,27 +48,15 @@ runs: shell: bash run: npm install -g npm@latest - - name: Install dependencies - shell: bash - env: - COREPACK_ENABLE_STRICT: '1' - COREPACK_ENABLE_DOWNLOAD_PROMPT: '0' - run: | - set -euo pipefail - if [ -f pnpm-lock.yaml ]; then - corepack enable - pnpm install --frozen-lockfile --ignore-scripts - elif [ -f yarn.lock ]; then - corepack enable - yarn install --immutable --mode=skip-build || yarn install --frozen-lockfile --ignore-scripts - elif [ -f package-lock.json ]; then - npm ci --ignore-scripts - else - npm install --ignore-scripts --no-package-lock - fi + - name: Download tarball artifact + uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + with: + name: uppt-tarball + path: ${{ runner.temp }}/uppt-tarballs - name: Publish to npm shell: bash env: NPM_ACCESS: ${{ inputs.npm-access }} + TARBALL_DIR: ${{ runner.temp }}/uppt-tarballs run: node --experimental-strip-types ${{ github.action_path }}/../scripts/publish.ts diff --git a/scripts/pack.ts b/scripts/pack.ts new file mode 100644 index 0000000..29dbc47 --- /dev/null +++ b/scripts/pack.ts @@ -0,0 +1,83 @@ +// Pack the current tag into one or more tarballs in `PACK_OUT_DIR`. +// The composite action then hands those tarballs to a workflow artifact +// upload step; the `publish` job in the same workflow run downloads the +// artifact and runs `npm stage publish` on the tarball without ever +// installing the package's dependencies. +// +// Two paths: +// - pnpm-lock.yaml present: `pnpm pack` so `catalog:` specifiers get +// resolved into the tarball. +// - otherwise: `npm pack`. +// +// Env: +// PACK_OUT_DIR directory to write the `*.tgz` into (created if +// missing). The action then uploads its contents as +// a workflow artifact. +// GITHUB_REF must be `refs/tags/v*` (set automatically) + +import process from 'node:process' +import { execFileSync } from 'node:child_process' +import { existsSync, mkdirSync, readdirSync, readFileSync, statSync } from 'node:fs' +import { resolve } from 'node:path' + +function run (cmd: string, args: string[], cwd?: string) { + console.log('$', cmd, ...args, cwd ? `(cwd: ${cwd})` : '') + execFileSync(cmd, args, { stdio: 'inherit', cwd }) +} + +function tarballGlobPrefix (pkgName: string): string { + // npm/pnpm pack names tarballs `-.tgz` for unscoped + // packages and `--.tgz` for scoped ones (the + // leading `@` is stripped and the `/` becomes `-`). + return pkgName.replace(/^@/, '').replace(/\//g, '-') +} + +function findTarballs (dir: string, prefix: string): string[] { + return readdirSync(dir) + .filter(f => f.startsWith(`${prefix}-`) && f.endsWith('.tgz')) + .sort() +} + +function main () { + const ref = process.env.GITHUB_REF ?? '' + if (!ref.startsWith('refs/tags/v')) { + throw new Error(`GITHUB_REF must be a 'refs/tags/v*' ref, got '${ref || ''}'`) + } + const tag = ref.slice('refs/tags/'.length) + if (!/^v\d+\.\d+\.\d+(?:-[\w.-]+)?(?:\+[\w.-]+)?$/.test(tag)) { + throw new Error(`Refusing to pack: tag "${tag}" is not strict semver`) + } + + const outDir = process.env.PACK_OUT_DIR + if (!outDir) throw new Error('PACK_OUT_DIR is required') + mkdirSync(outDir, { recursive: true }) + + const pkgPath = resolve(process.cwd(), 'package.json') + const pkg = JSON.parse(readFileSync(pkgPath, 'utf8')) as { name: string } + const hasPnpmLock = existsSync(resolve(process.cwd(), 'pnpm-lock.yaml')) + + if (hasPnpmLock) { + run('pnpm', ['pack', '--pack-destination', outDir]) + } else { + run('npm', ['pack', '--pack-destination', outDir]) + } + + const prefix = tarballGlobPrefix(pkg.name) + const tarballs = findTarballs(outDir, prefix) + if (!tarballs.length) { + throw new Error(`No tarball matching ${prefix}-*.tgz found in ${outDir} after pack`) + } + + for (const tarball of tarballs) { + const size = statSync(resolve(outDir, tarball)).size + console.log(`Packed ${tarball} (${size} bytes) for ${tag}`) + } +} + +try { + main() +} +catch (err) { + console.error(err) + process.exit(1) +} diff --git a/scripts/pin-readme.ts b/scripts/pin-readme.ts index 9aec1ad..d456b9f 100644 --- a/scripts/pin-readme.ts +++ b/scripts/pin-readme.ts @@ -39,7 +39,7 @@ function rewrite (readme: string, sha: string, tag: string) { // Match `uses: danielroe/uppt/@` optionally followed by a `# // ` trailing comment. `` is restricted to the three known // subactions so we don't accidentally rewrite future siblings. - const pinRe = /(\buses:\s*danielroe\/uppt\/(?:pr|release|publish))@\S+(?:\s+#\s*v\S+)?/g + const pinRe = /(\buses:\s*danielroe\/uppt\/(?:pr|release|pack|publish))@\S+(?:\s+#\s*v\S+)?/g return readme.replace(pinRe, (_match, prefix: string) => `${prefix}@${sha} # ${tag}`) } diff --git a/scripts/publish.ts b/scripts/publish.ts index 6a0c6dd..9a336fa 100644 --- a/scripts/publish.ts +++ b/scripts/publish.ts @@ -1,17 +1,22 @@ -// Stage-publish to npm using OIDC trusted publishing. The maintainer -// approves the staged version with 2FA on npmjs.com afterwards. +// Stage-publish prebuilt tarball(s) to npm using OIDC trusted +// publishing. The maintainer approves the staged version with 2FA on +// npmjs.com afterwards. // -// Two paths, differing only in what gets staged: -// - pnpm-lock.yaml present: `pnpm pack` first so `catalog:` specifiers -// resolve in the tarball, then `npm stage publish ./.tgz`. -// - otherwise: `npm stage publish` from source (no tarball arg). +// The tarball(s) were produced by `uppt/pack` in an earlier job in the +// same workflow run and downloaded into `TARBALL_DIR` by +// `actions/download-artifact`. +// +// `npm publish ` doesn't run lifecycle scripts in any case +// (the tarball is treated as an opaque artifact), but we still pass +// `--ignore-scripts` for clarity. // // Env: // NPM_ACCESS `public` (default) or `restricted` +// TARBALL_DIR directory holding the prebuilt `*.tgz` files import process from 'node:process' import { execFileSync } from 'node:child_process' -import { existsSync, readFileSync, readdirSync } from 'node:fs' +import { existsSync, readdirSync } from 'node:fs' import { resolve } from 'node:path' function run (cmd: string, args: string[]) { @@ -19,39 +24,25 @@ function run (cmd: string, args: string[]) { execFileSync(cmd, args, { stdio: 'inherit' }) } -function tarballGlobPrefix (pkgName: string): string { - // npm pack names tarballs `-.tgz` for unscoped packages - // and `--.tgz` for scoped ones (the leading `@` - // is stripped and the `/` becomes `-`). - return pkgName.replace(/^@/, '').replace(/\//g, '-') -} - -function findTarballs (prefix: string): string[] { - return readdirSync(process.cwd()) - .filter(f => f.startsWith(`${prefix}-`) && f.endsWith('.tgz')) - .sort() +function findTarballs (dir: string): string[] { + return readdirSync(dir).filter(f => f.endsWith('.tgz')).sort() } function main () { const access = process.env.NPM_ACCESS === 'restricted' ? 'restricted' : 'public' - const pkgPath = resolve(process.cwd(), 'package.json') - const pkg = JSON.parse(readFileSync(pkgPath, 'utf8')) as { name: string } - const hasPnpmLock = existsSync(resolve(process.cwd(), 'pnpm-lock.yaml')) - if (!hasPnpmLock) { - run('npm', ['stage', 'publish', '--provenance', `--access=${access}`]) - return - } - - run('pnpm', ['pack', '--pack-destination', '.']) + const dir = process.env.TARBALL_DIR + if (!dir) throw new Error('TARBALL_DIR is required') + if (!existsSync(dir)) throw new Error(`TARBALL_DIR does not exist: ${dir}`) - const prefix = tarballGlobPrefix(pkg.name) - const tarballs = findTarballs(prefix) + const tarballs = findTarballs(dir) if (!tarballs.length) { - throw new Error(`No tarball matching ${prefix}-*.tgz found in ${process.cwd()}`) + throw new Error(`No *.tgz found in ${dir}. Did the pack job upload the artifact?`) } + for (const tarball of tarballs) { - run('npm', ['stage', 'publish', `./${tarball}`, '--provenance', `--access=${access}`]) + const tarballPath = resolve(dir, tarball) + run('npm', ['stage', 'publish', tarballPath, '--provenance', '--ignore-scripts', `--access=${access}`]) } } -- 2.51.2