v0.2: pnpm support in npm build type (single-package only) by TomHennen · Pull Request #212 · TomHennen/wrangle

TomHennen · 2026-05-13T02:04:41Z

Summary

Closes #207. Extends the npm build type to support pnpm single-package projects, detected by pnpm-lock.yaml. Workspaces are still rejected (tracked in #208 / PR #211 research). Yarn is still rejected explicitly with a clear error.

Pattern mirrors python's uv work: same composite action, one new branch in install/pack scripts, same SBOM + hashing + reusable-workflow wiring.

What's in the diff

build/actions/npm/validate_inputs.sh — accept pnpm-lock.yaml; reject yarn.lock with a "file an issue if you need it" hint; reject the ambiguous case where both package-lock.json and pnpm-lock.yaml exist (so wrangle never silently picks one and attests the wrong build). Workspaces detection unchanged — still rejected, with the message pointing at v0.2: npm/pnpm/yarn workspaces support (N-tarball artifact model) #208.
build/actions/npm/build_and_pack.sh — detect the package manager from the lockfile, then branch: pnpm install --frozen-lockfile + pnpm pack --pack-destination dist for pnpm; npm ci + npm pack --pack-destination dist for npm. --ignore-scripts and run-tests semantics extend identically across both managers.
build/actions/npm/action.yml — Detect tooling step now emits package-manager and a conditional cache output. Setup Node.js consumes the dynamic cache (empty string for pnpm; 'npm' for npm). New conditional Setup pnpm (via Corepack) step runs only when pnpm is detected; uses the bundled-with-Node Corepack to activate whatever pnpm version package.json#packageManager pins.
build/actions/npm/test.bats — adds tests for: pnpm acceptance, yarn rejection, ambiguous-lockfile rejection, PM detection in build_and_pack, conditional cache config, no pnpm-store cache (per When pnpm/Yarn support lands, do NOT enable pnpm-store / yarn cache (Mini Shai-Hulud lesson) #205), Corepack enablement gated on pnpm, and a structural assertion that scripts are invoked via bash <script> (see security note below).
build/actions/npm/README.md — adds a "Caching" section explaining the npm-cache-safe vs pnpm-cache-disabled asymmetry; updates the "v0.1 limitations" block to a "v0.2 status" block; notes WORKSPACES_PHASE_1.md as the workspaces follow-on.

Security review

The cache poisoning lesson from May 2026's Mini Shai-Hulud / TanStack compromise (#205) is the load-bearing decision here:

No pnpm-store cache, ever. setup-node's cache: 'pnpm' caches ~/.local/share/pnpm/store — content-addressed paths whose claimed hash pnpm does NOT re-verify against actual bytes at install time. An attacker with code execution in any workflow on the repo can poison the store; subsequent legitimate runs hardlink the malicious bytes into node_modules/ and produce a faithfully-signed malicious build. The Detect tooling step explicitly emits an empty cache= output on the pnpm path and Setup Node.js consumes it dynamically. A bats test locks this in (refuses pnpm cache).
The npm path keeps cache: 'npm'. Safe because npm ci re-validates each cached tarball's integrity against package-lock.json on every install — a poisoned ~/.npm cache entry whose bytes don't hash to the lockfile's pinned integrity is rejected before extraction. The threat-model comment in action.yml now documents the asymmetry explicitly.
Lockfile-faithful install on both paths. npm ci aborts on lockfile drift; pnpm install --frozen-lockfile aborts on lockfile drift. Wrangle never falls back to npm install / pnpm install without --frozen-lockfile, so adopters can't accidentally attest a build whose resolved deps differ from the committed lockfile.
Corepack trust model. Corepack downloads pnpm from the npm registry on first use and verifies it via the registry's signed metadata — same trust chain as npm install itself. Not hermetic, but no weaker than every other tool wrangle reaches for.
ignore-scripts threading. Passed to both install AND pack on both managers; <pm> run build / <pm> test are skipped outright when set. Adopters who want the strict "source bytes only, no script execution" attestation get identical semantics on npm and pnpm.
Ambiguous-lockfile rejection. If both package-lock.json and pnpm-lock.yaml exist, validate_inputs.sh fails fast with the cleanup hint. Silently picking one would silently determine what gets attested.
Yarn rejection is explicit. A yarn.lock produces a clear error with a "file an issue if you need Yarn support" hint, rather than the confusing "no lockfile found" the v0.1 code would have produced.

One regression introduced + fixed in this PR

The .sh files lost their 0755 exec bit on the first push (the GitHub MCP push_files API used to maintain this branch demotes mode to 0644 on overwrite). I cannot restore the bit through the MCP API — push_files / create_or_update_file don't expose a mode parameter.

Fix: action.yml now invokes the scripts via bash "${{ github.action_path }}/<script>" ... rather than calling them directly. Functionally equivalent given the #!/bin/bash shebangs, and defensive against this and any similar future regression. The bats suite has a new structural test (asserts scripts are invoked via 'bash <script>') that locks the new pattern in.

If you want to revert to direct invocation matching python's pattern, the path is: chmod +x build/actions/npm/{validate_inputs,build_and_pack}.sh locally + commit + push, then drop the bash prefix in action.yml. The bats test can be updated in the same commit.

Test plan

CI runs the bats suite (build/actions/npm/test.bats) on this PR — all new tests should pass.
Source-scan (zizmor, OSV, scorecard) clean — no new actions, no new permissions.
Reviewer: gut-check the cache= dynamic-output pattern in action.yml. setup-node treats empty string as "no caching"; if that's no longer true in a future setup-node version we'd want to assert it via test, not assume it.
Integration test in wrangle-test against a real pnpm fixture — separate PR (uses the existing @tomhennen/wrangle-integration-fixture package; build-and-verify only, no publish).

Closes v0.2: pnpm support (single-package only; workspaces tracked separately) #207
Honors When pnpm/Yarn support lands, do NOT enable pnpm-store / yarn cache (Mini Shai-Hulud lesson) #205 (no pnpm-store cache)
Workspaces (v0.2: npm/pnpm/yarn workspaces support (N-tarball artifact model) #208) deferred to PR Phase 1 research: npm workspaces support #211 research + its implementation follow-up
Yarn support deferred to a future issue if there's demand
pull_request_target refusal guard (Refuse pull_request_target invocations in every reusable workflow #202) is a separate companion PR

https://claude.ai/code/session_01AqkojrFQsx5CgHGndN96E2

Generated by Claude Code

Extends the existing npm build action to handle pnpm projects. Single-package only — workspaces support is tracked separately in #208 (design doc in WORKSPACES_PHASE_1.md, also on this branch's predecessor). Files changed: build/actions/npm/validate_inputs.sh: - Accept pnpm-lock.yaml as a valid lockfile. - Still reject yarn.lock (Yarn support is a follow-on). - New: reject ambiguous state (both npm AND pnpm lockfiles present) rather than silently picking one. - Workspaces rejection updated to point at #208 tracking issue. build/actions/npm/build_and_pack.sh: - Detect package manager from lockfile (pnpm-lock.yaml -> pnpm, otherwise npm). - Use pnpm install --frozen-lockfile / pnpm pack on the pnpm path; npm ci / npm pack on the npm path. ignore-scripts threading works the same for both managers. - Build / test detection logic unchanged — uses jq to read package.json scripts and invokes via "$PM" run / "$PM" test. build/actions/npm/action.yml: - "Detect Node.js version source" step renamed to "Detect tooling"; now also outputs package-manager and conditional cache config. - Setup Node.js's `cache` input is now driven dynamically by the tooling step's output. npm path -> 'npm'; pnpm path -> empty (no caching). - New "Setup pnpm (via Corepack)" step gated on package-manager == 'pnpm'. Corepack reads packageManager from package.json if set, otherwise uses its bundled default. - Comment block at the tooling step's pnpm branch explicitly documents the no-pnpm-store-cache decision and references #205. - New package-manager output for downstream visibility. - Build summary now includes which package manager was used. build/actions/npm/test.bats: - Updated tests for pnpm acceptance (the previous "rejects pnpm and yarn" test is now split: "accepts pnpm" + "rejects yarn"). - New tests covering: ambiguous-lockfile rejection, package-manager detection in build_and_pack.sh, conditional cache config in action.yml, no-pnpm-cache rule (#205), Corepack enablement on the pnpm path. - Updated ignore-scripts threading test to handle both npm and pnpm install/pack invocations. - Workspaces-rejection test now verifies the error message points at #208. build/actions/npm/README.md: - Updated intro to mention both npm and pnpm. - "What this action does" section reflects both managers. - New "Caching" section explaining the asymmetry (npm cache safe, pnpm cache deliberately disabled per #205). - "v0.1 limitations" renamed to "v0.2 status"; pnpm removed from the unsupported list. Workspaces rejection now points at #208 and the new WORKSPACES_PHASE_1.md. Security review notes: - Corepack downloads pnpm from the npm registry; integrity is verified via npm's signed metadata. Same trust chain as npm install. - pnpm install --frozen-lockfile is lockfile-faithful and verifies integrity hashes at fetch time, equivalent to npm ci. - pnpm-store cache is deliberately not enabled (tracked in #205, enforced by bats test). - ignore-scripts flag threads through to both managers identically. - No new SHA-pinned third-party actions; reuses actions/setup-node and the existing Corepack bundled with Node 16.10+. #207

…mode loss) The GitHub MCP push_files API used to maintain this branch does NOT preserve the executable bit when overwriting existing files. When the v0.2 commit (7e72d33) overwrote validate_inputs.sh and build_and_pack.sh with pnpm-support content, both files were demoted from -rwxr-xr-x (755) to -rw-r--r-- (644). The action.yml's previous pattern of invoking the scripts directly (e.g., "${{ github.action_path }}/foo.sh") would fail with "Permission denied" on the runner. Fix: explicit `bash <script>` invocation. Works regardless of mode; the scripts retain their #!/bin/bash shebang for local-shell use. Slight deviation from build/actions/python/action.yml's direct- invocation pattern, but it's the defensive choice and avoids relying on push_files behavior. Additional bats test asserting the bash-invocation pattern is used on both scripts so the regression is caught structurally. Dropped the "exists and is executable" tests in favor of "exists" since exec bit is no longer functionally required. Added a comment in test.bats and action.yml explaining the rationale. A follow-on cleanup (probably from a maintainer's terminal where chmod can be applied via git directly) could restore the exec bit and revert action.yml to direct invocation, which would match the python action's pattern. Not required for v0.2 functional correctness.

Adds a parallel pnpm/ fixture exercising wrangle's pnpm tooling branch end-to-end through build_and_publish_npm.yml: Corepack-driven pnpm activation, `pnpm install --frozen-lockfile`, `pnpm pack`, SBOM, hash computation, SLSA generic provenance, and slsa-verifier verification. The pnpm fixture reuses the existing @tomhennen/wrangle-integration-fixture package name so no new Trusted Publisher registration is needed on npmjs.com. It's intentionally build-only — publish-to-npmjs.org is already covered by test-npm via the same package name, and doing the publish twice in one run would race on the `--tag integration` version. Mirrors test-python-uv's "double up coverage on one fixture" pattern (uv tooling + verify-opt-out on a single fixture). The lockfile is generated by pnpm 9.15.4 against ms@2.1.3 — same single zero-transitive-dep package as npm/, so the pnpm path exercises the same SBOM/integrity machinery as the npm path. The `packageManager` field pins the pnpm version Corepack activates deterministically. Pairs with TomHennen/wrangle#212 (pnpm support in the npm build type).

The test that enforces "never use pnpm-store cache" was matching its own documentation in action.yml — a comment explaining the avoided pattern contains the literal string `cache: 'pnpm'`. Mirror test 264's anchored regex so the assertion catches only actual YAML keys, not prose. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Required so wrangle-test PR #9's test-pnpm job actually exercises the new composite action: the reusable workflow's inner `uses: ...@<sha>` pins resolve to a SHA on this branch, not main's pre-pnpm SHA. Only the npm pin is load-bearing for pnpm coverage; the other four workflows bump along as a no-op per `tools/bump_action_pins.sh`'s all-or-nothing design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

TomHennen · 2026-05-14T00:17:44Z

-    # extraction. setup-node's `cache: 'npm'` caches `~/.npm` only — it
-    # does NOT cache `node_modules/`, which would bypass integrity checks
-    # if cached as extracted modules.
+        # Package manager + cache configuration.


has this whole section gotten to the point of "should be a shell script"?

Maybe it doesn't make sense for GITHUB_OUTPUT stuff?

TomHennen · 2026-05-14T00:19:20Z

      run: |
        set -euo pipefail
-        "${{ github.action_path }}/build_and_pack.sh" "$INPUT_PATH" "$RUN_TESTS" "$IGNORE_SCRIPTS"
+        bash "${{ github.action_path }}/build_and_pack.sh" "$INPUT_PATH" "$RUN_TESTS" "$IGNORE_SCRIPTS"


do we need this bash prefix? I think that was because the script had been set to 0644 incorrectly. This problem might exist elsehwere in this PR too?

TomHennen · 2026-05-14T00:21:31Z

 runs:
  using: "composite"
  steps:
+    # Scripts are invoked via `bash <script>` (rather than directly) so


See other comments, this is a hack we don't need. No one cares about the MCP API limitation.

TomHennen · 2026-05-14T00:22:40Z

+            printf 'Detected npm lockfile; using npm with cache=npm.\n'
+        fi
+
+    # Cache npm's registry tarball cache (~/.npm), keyed on the lockfile,


Are all these comments about why a bit much? is it well covered already, in this file?

…plicate comments - Restore 0755 on validate_inputs.sh and build_and_pack.sh; drop the `bash <script>` workaround and the comment explaining it. - Extract the Detect tooling step's inline shell into detect_tooling.sh, matching the validate_inputs.sh / build_and_pack.sh pattern. - Drop the duplicate caching comment block at Setup Node.js — the rationale lives in detect_tooling.sh and README.md. - Update bats: assert exec bit (-x) on the scripts like python/ does, add a test for detect_tooling.sh, drop the bash-prefix assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Add npm fixture and test-npm job for wrangle integration tests The npm fixture mirrors the existing python/ and container/ shape: a single-package, no-dependency project that exercises wrangle's build_and_publish_npm.yml end-to-end (build, SBOM, hash, gate, provenance, verify). Publish is intentionally NOT wired up — npm Trusted Publishing cannot mint a package's first version (npm/cli#8544) and the trusted publisher needs to be registered on npmjs.com first. Both are human-only bootstrap steps. The package-lock.json is hand-written (lockfileVersion 3, no deps) so no npm install is needed. The package name is the stable name we'll eventually register Trusted Publishing against. * Add ms dependency to npm fixture for SBOM/npm-ci coverage Without a dep, npm ci is a no-op and the syft SBOM only sees the root package, so we're not actually exercising those code paths. ms@2.1.3 is a stable, zero-transitive-dep, ~5KB pure-JS package (no native code) — enough to make the SBOM meaningful and verify npm ci installs from the lockfile, without dragging in a dep tree that adds lockfile churn. Lockfile generated by `npm install` (npm 10.9.7 / node v22.22.2). * Add pnpm fixture and test-pnpm integration job Adds a parallel pnpm/ fixture exercising wrangle's pnpm tooling branch end-to-end through build_and_publish_npm.yml: Corepack-driven pnpm activation, `pnpm install --frozen-lockfile`, `pnpm pack`, SBOM, hash computation, SLSA generic provenance, and slsa-verifier verification. The pnpm fixture reuses the existing @tomhennen/wrangle-integration-fixture package name so no new Trusted Publisher registration is needed on npmjs.com. It's intentionally build-only — publish-to-npmjs.org is already covered by test-npm via the same package name, and doing the publish twice in one run would race on the `--tag integration` version. Mirrors test-python-uv's "double up coverage on one fixture" pattern (uv tooling + verify-opt-out on a single fixture). The lockfile is generated by pnpm 9.15.4 against ms@2.1.3 — same single zero-transitive-dep package as npm/, so the pnpm path exercises the same SBOM/integrity machinery as the npm path. The `packageManager` field pins the pnpm version Corepack activates deterministically. Pairs with TomHennen/wrangle#212 (pnpm support in the npm build type). * Sync stale branch state to main (no-op for the pnpm PR diff) The npm/package.json on this branch is missing the repository field added on main in c88dc12 (Trusted Publishing provenance check requires it), and the container-no-verify/ fixture added on main in 0fb46a4 isn't here either. Both predate the pnpm work but were absent because this branch forked before they landed. Pushing main's current content for these paths avoids the pnpm PR diff showing spurious "reverts" of those changes.

TomHennen added 2 commits May 12, 2026 21:56

TomHennen temporarily deployed to integration-test May 13, 2026 02:04 — with GitHub Actions Inactive

TomHennen mentioned this pull request May 13, 2026

Add pnpm fixture + test-pnpm integration job TomHennen/wrangle-test#9

Merged

3 tasks

TomHennen temporarily deployed to integration-test May 13, 2026 23:44 — with GitHub Actions Inactive

TomHennen temporarily deployed to integration-test May 14, 2026 00:13 — with GitHub Actions Inactive

TomHennen commented May 14, 2026

View reviewed changes

TomHennen had a problem deploying to integration-test May 14, 2026 00:41 — with GitHub Actions Error

Re-bump pins to post-refactor SHA so integration tests cover final code

75519cd

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

TomHennen temporarily deployed to integration-test May 14, 2026 00:42 — with GitHub Actions Inactive

TomHennen merged commit a84c855 into main May 14, 2026
6 checks passed

TomHennen deleted the claude/v0.2-pnpm-support branch May 14, 2026 01:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v0.2: pnpm support in npm build type (single-package only)#212

v0.2: pnpm support in npm build type (single-package only)#212
TomHennen merged 6 commits into
mainfrom
claude/v0.2-pnpm-support

TomHennen commented May 13, 2026

Uh oh!

TomHennen May 14, 2026

Uh oh!

TomHennen May 14, 2026

Uh oh!

TomHennen May 14, 2026

Uh oh!

TomHennen May 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

TomHennen commented May 13, 2026

Summary

What's in the diff

Security review

One regression introduced + fixed in this PR

Test plan

Related

Uh oh!

TomHennen May 14, 2026

Choose a reason for hiding this comment

Uh oh!

TomHennen May 14, 2026

Choose a reason for hiding this comment

Uh oh!

TomHennen May 14, 2026

Choose a reason for hiding this comment

Uh oh!

TomHennen May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant