Skip to content

build: install pnpm via npm instead of corepack - #4948

Merged
escapedcat merged 1 commit into
masterfrom
build/drop-corepack
Aug 16, 2026
Merged

build: install pnpm via npm instead of corepack#4948
escapedcat merged 1 commit into
masterfrom
build/drop-corepack

Conversation

@escapedcat

Copy link
Copy Markdown
Member

Description

Replace corepack enable with an explicit npm install of the pnpm version pinned in the packageManager field, in both Dockerfiles, and drop the corepack suggestion from CONTRIBUTING.md (mise and direct install remain).

Motivation and Context

pnpm now explicitly discourages installing via corepack — the "Using Corepack" section was removed from pnpm.io/installation (pnpm/pnpm.io#862):

Usage examples

Not applicable — no runtime behavior changes; the published image contents are identical.

How Has This Been Tested?

Built Dockerfile.ci from a pristine clone of this branch (the same conditions container-build.yml runs under): image builds green end-to-end, including the final-stage self-test (TypeScript config loads, over-length header correctly rejected).

Note: Dockerfile.dev is unbuildable before and after this change — its brainpower/node-cubicle base image predates corepack entirely (exit 127 on master). A follow-up PR retires it.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have verified that any documentation examples I added/changed actually work.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • For a feature/bug fix, my commits follow the test-driven flow: a failing-test commit, then the implementation.

🤖 Generated with Claude Code

Corepack is no longer distributed with Node.js 25+ and cannot install
pnpm >= 12, so install the version pinned in the `packageManager` field
via npm instead — the same approach CI.yml already uses. Also drop the
corepack suggestion from CONTRIBUTING.md; any installed pnpm >= 10
switches to the pinned version on its own.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The PR appears safe to merge, with a non-blocking supply-chain hardening concern because Docker builds no longer enforce the pnpm integrity hash pinned in package.json.

The changed commands install the intended exact pnpm version, but they strip its pinned SHA-512 value and therefore no longer enforce the repository’s explicit artifact-integrity constraint.

Files Needing Attention: Dockerfile.ci and Dockerfile.dev

Security Review

The npm bootstrap preserves the numeric pnpm version but discards the packageManager SHA-512 pin, weakening artifact-integrity verification for image builds. How this was verified: The packageManager value includes a SHA-512 suffix, while both changed npm commands strip that suffix before installation.

Important Files Changed

Filename Overview
Dockerfile.ci Replaces Corepack with an npm global install that selects the correct pnpm version but omits the packageManager integrity pin.
Dockerfile.dev Applies the same npm bootstrap to the legacy development image; its pre-existing obsolete-base build failure is explicitly outside this PR.
.github/CONTRIBUTING.md Removes the Corepack recommendation while retaining mise and direct-install guidance.

Reviews (1): Last reviewed commit: "build: install pnpm via npm instead of c..." | Re-trigger Greptile

Comment thread Dockerfile.ci
RUN corepack enable && \
# Install the pnpm version pinned in package.json `packageManager` via npm;
# corepack was removed from Node 25+ and cannot install pnpm >= 12.
RUN npm install --global pnpm@$(node -p "require('./package.json').packageManager.split('@')[1].split('+')[0]") && \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Preserve the pnpm integrity pin

The new parser strips the +sha512 value from packageManager, so Docker builds enforce only the pnpm version and no longer validate the repository's pinned artifact integrity, weakening reproducibility and supply-chain verification. The same pattern is also present in Dockerfile.dev.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@escapedcat
escapedcat added this pull request to the merge queue Aug 16, 2026
Merged via the queue into master with commit 8647102 Aug 16, 2026
16 checks passed
@escapedcat
escapedcat deleted the build/drop-corepack branch August 16, 2026 09:13
pull Bot pushed a commit to TheRakeshPurohit/commitlint that referenced this pull request Aug 16, 2026
…ventional-changelog#4952)

The corepack replacement in conventional-changelog#4948 installed pnpm by version only,
dropping the integrity check corepack performed against the +sha512
suffix of the `packageManager` field. Download the tarball with
`npm pack`, check it against the pin (the pin is the sha512 hex digest
of the npm tarball), and install from the verified file.

The tarball is removed after install so the final stage's *.tgz glob
keeps matching only the packed commitlint packages.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant