fix: pnpm settings survive the pnpm 11 bump - #29
Merged
Conversation
pnpm 11 does not read the `pnpm` field in package.json, and it does not fail when it finds one — it prints "The following keys were ignored: pnpm.overrides" and installs anyway. Four CVE floors live in that field here, so the pending pnpm 10 → 11 bump would have dropped all four and relocked the tree onto whatever resolves naturally, with a green install. Upstream tracks this as a security-affecting silent ignore (pnpm/pnpm#11536). osv-scanner is a required check in this repository and would have caught the CVE-driven floors coming back, but `nanoid: ^3.3.17` is not a CVE floor in the direction that matters — the caret is what keeps resolution inside the 3.x line, because `>=3.3.17` resolves to nanoid 6, which is ESM-only and is not the API postcss calls. OSV has no opinion on that, so that one would have gone through. pnpm 10 already reads pnpm-workspace.yaml, so the settings move now, ahead of the bump, rather than as part of it. No window exists where they are read from neither place. The move also lets each floor carry its own advisory and the reason for its exact form — why fast-uri is `>=4.1.2` and not the 3.x patch level, why js-yaml is scoped to the 4.x line so astro's own 5.x parser is left alone, why nanoid is a caret. That rationale was in commit bodies and nowhere in the file; JSON had no place to put it. Verified: the lockfile is byte-identical before and after the move, and the check is not vacuous — with the overrides removed the same command downgrades fast-uri 4.1.2 → 3.1.5, so an unchanged lockfile means the floors are still being applied rather than that nothing re-resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four CVE floors were about to be dropped on the floor by a routine package-manager bump.
What happens without this
pnpm 11 does not read the
pnpmfield inpackage.json, and it does not fail when it finds one. It prints…and installs anyway. That warning is already in this repo's CI log on #27, the pending pnpm 10 → 11 bump. Upstream tracks the silent-ignore behaviour as security-affecting: pnpm/pnpm#11536.
osv-scanneris a required check here and would have caught the CVE-driven floors coming back. The one it would not catch isnanoid: ^3.3.17— the caret, not the floor, is the load-bearing part.>=3.3.17resolves to nanoid 6, which is ESM-only and is not the API postcss calls. OSV has no opinion on a compatibility pin, so that one goes through silently and surfaces later as a build failure with no obvious cause.Why now and not with the bump
pnpm 10.32.1 already reads
pnpm-workspace.yaml. Verified locally, not assumed. Landing the move first means there is no window in which the settings are read from neither place, and it leaves #27 as a pure package-manager bump.onlyBuiltDependencies→allowBuildsis a different story — pnpm 10 silently ignoresallowBuilds— so that rename has to happen with a bump, not before. This repo has no build-script settings, so it does not apply here.What the move buys beyond correctness
Each floor now carries its advisory and the reason for its exact form: why
fast-uriis>=4.1.2and not the 3.x patch level, whyjs-yamlis scoped to the4line so astro's own 5.x parser is left alone, whynanoidis a caret. All of that was in commit bodies and none of it was in the file. JSON had nowhere to put it.Verification
pnpm install --lockfile-onlyafter the move produces a lockfile byte-identical tomain.fast-uri4.1.2 → 3.1.5 and rewrites theoverrides:block. So "identical" means the floors are still applied, not that nothing re-resolved.pnpm lintexits 0.osv-scannersees the same input asmain.Note on #27
Its remaining failure is unrelated to this: pnpm 11 changed the
minimumReleaseAgedefault from0to1440, andnanoid@3.3.18published inside that window. It clears on its own once the package is 24h old. Merging this movesmain, which rebases #27 and re-runs its checks.