Fix/remove unused deps supply chain#988
Conversation
✅ Deploy Preview for gardener-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
gardener-prow
Bot
added
the
cla: yes
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR migrates tooling from npm/npx to pnpm: config and ignore rules updated, package manifests and workspace adjusted (pins/overrides), Dockerfile and Makefile switched to pnpm commands, Vite config pre-bundles dayjs, and blog sidebar sorting was refactored to remove lodash. Changespnpm Package Manager Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gardener-prow
Bot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.vitepress/theme/blog-sidebar.ts (2)
24-25: ⚡ Quick winAdd validation for numeric year values.
If
textcontains non-numeric values,parseIntreturnsNaN, causing unpredictable sort behavior. While year folders should always be numeric, adding validation improves robustness.🛡️ Proposed defensive implementation
sidebar['/blog/'].items = [...sidebar['/blog/'].items] - .sort((a: any, b: any) => parseInt(b.text) - parseInt(a.text)) + .sort((a: any, b: any) => { + const aYear = parseInt(a.text, 10) + const bYear = parseInt(b.text, 10) + if (isNaN(aYear) || isNaN(bYear)) return 0 + return bYear - aYear + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.vitepress/theme/blog-sidebar.ts around lines 24 - 25, The sort callback on sidebar['/blog/'].items uses parseInt directly and can yield NaN for non-numeric text; update the comparator to parse with a radix, validate the results (e.g., const ay = parseInt(a.text, 10); const by = parseInt(b.text, 10);) and replace NaN with a safe fallback (such as 0 or -Infinity to push non-years to the end) before subtracting, so the comparator (used on sidebar['/blog/'].items .sort(...)) always returns a numeric value and yields stable ordering.
35-36: ⚡ Quick winAdd validation for numeric month values.
Similar to the year folder sorting above, month folder sorting should validate numeric values to prevent
NaNcomparison issues.🛡️ Proposed defensive implementation
yearItem.items = [...yearItem.items] - .sort((a: any, b: any) => parseInt(b.text) - parseInt(a.text)) + .sort((a: any, b: any) => { + const aMonth = parseInt(a.text, 10) + const bMonth = parseInt(b.text, 10) + if (isNaN(aMonth) || isNaN(bMonth)) return 0 + return bMonth - aMonth + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.vitepress/theme/blog-sidebar.ts around lines 35 - 36, The month-folder sorting needs numeric validation like the year sorting: when comparing monthItem.items (and analogous to yearItem.items) parse each item's text into a number, check with Number.isNaN and handle NaN safely (e.g., treat non-numeric as a fallback value such as 0 or -Infinity) before performing the descending numeric comparison; update the comparator used for monthItem.items to compute parsedA and parsedB, validate them, and then return parsedB - parsedA so NaN values don’t produce incorrect ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Line 103: The install target currently runs `pnpm install` without
`--frozen-lockfile`, so update the Makefile to ensure CI uses a frozen lockfile:
either change the existing install target (symbol: install) to run `pnpm install
--frozen-lockfile` so all invocations (including ci-build) are deterministic, or
add a new target (e.g., ci-install) that runs `pnpm install --frozen-lockfile`
and update the ci-build target (symbol: ci-build) to call ci-install instead of
install; pick one approach and keep the other target behavior unchanged to avoid
breaking local developer workflows.
In `@package.json`:
- Line 23: Update the pinned dependency "axios" in package.json from "1.14.0" to
a secure version (>= 1.16.0) to include fixes for the HIGH and MODERATE
advisories, then regenerate the lockfile (npm install / npm ci or yarn install)
to refresh package-lock.json or yarn.lock and commit the updated lockfile;
optionally run npm audit or yarn audit to verify no remaining vulnerabilities.
---
Nitpick comments:
In @.vitepress/theme/blog-sidebar.ts:
- Around line 24-25: The sort callback on sidebar['/blog/'].items uses parseInt
directly and can yield NaN for non-numeric text; update the comparator to parse
with a radix, validate the results (e.g., const ay = parseInt(a.text, 10); const
by = parseInt(b.text, 10);) and replace NaN with a safe fallback (such as 0 or
-Infinity to push non-years to the end) before subtracting, so the comparator
(used on sidebar['/blog/'].items .sort(...)) always returns a numeric value and
yields stable ordering.
- Around line 35-36: The month-folder sorting needs numeric validation like the
year sorting: when comparing monthItem.items (and analogous to yearItem.items)
parse each item's text into a number, check with Number.isNaN and handle NaN
safely (e.g., treat non-numeric as a fallback value such as 0 or -Infinity)
before performing the descending numeric comparison; update the comparator used
for monthItem.items to compute parsedA and parsedB, validate them, and then
return parsedB - parsedA so NaN values don’t produce incorrect ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 11ed7d82-6a9f-4cf5-b551-ad0002661f8a
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.dockerignore.gitignore.npmrc.vitepress/theme/blog-sidebar.tsDockerfileMakefilepackage.json
klocke-io
left a comment
klocke-io
left a comment
There was a problem hiding this comment.
Thanks for pushing this topic 🎉
this looks really good. Only Issue I had while testing ->
Currently, pnpm vitepress dev is not properly bringing up the page for me :)
| @@ -1 +1,3 @@ | |||
| min-release-age=7 | |||
| frozen-lockfile=true | |||
| minimum-release-age=10080 | |||
There was a problem hiding this comment.
consider adding
| minimum-release-age=10080 | |
| blockExoticSubdeps=true | |
| trustPolicy=no-downgrade |
| min-release-age=7 | ||
| frozen-lockfile=true | ||
| minimum-release-age=10080 | ||
| shamefully-hoist=true |
There was a problem hiding this comment.
check if we could drop
| shamefully-hoist=true | |
| - shamefully-hoist=true |
If we add
"node-addon-api": "8.8.0",
"node-gyp": "12.3.0",
in package.json
gardener-prow
Bot
added
the
needs-rebase
- Switch package manager from npm to pnpm (packageManager field, Makefile, Dockerfile) - Remove unused dependencies: autoprefixer and lodash-es - Replace _.orderBy() in blog-sidebar.ts with native Array.sort() - Pin all dependency versions exactly (no ^ or ~), including axios override - Add pnpm security config in .npmrc: minimum-release-age=10080, frozen-lockfile - Restrict postinstall scripts via onlyBuiltDependencies: [esbuild, sharp] - Enforce frozen lockfile in Dockerfile (pnpm install --frozen-lockfile) - Add pnpm-lock.yaml, remove package-lock.json - Exclude package-lock.json from git to prevent accidental npm usage
VitePress docs recommend shamefully-hoist=true with pnpm to ensure all transitive deps are accessible to Vite's module resolver. Without it, vitepress-plugin-mermaid's transitive deps (cytoscape, dayjs, debug) are not resolvable at the project root in a clean Docker build, causing a blank page. debug is additionally pinned as a direct dep since it is not reachable via shamefully-hoist alone.
shamefully-hoist=true in .npmrc was silently ignored because .dockerignore excluded it. Without it pnpm runs in strict mode inside the container and transitive deps required by vitepress-plugin-mermaid are not resolvable, causing a blank page.
- Add ci-install target with --frozen-lockfile; update ci-build to use it instead of install, keeping local developer workflow unchanged - Upgrade axios override from 1.14.0 to 1.16.1 to address HIGH/MODERATE security advisories fixed in 1.15.1, 1.15.2, and 1.16.0
- Upgrade packageManager to pnpm@11.5.1 - Move pnpm overrides and onlyBuiltDependencies from package.json to pnpm-workspace.yaml (required by pnpm 11) - Add allowBuilds for esbuild and sharp in pnpm-workspace.yaml - Replace shamefully-hoist with explicit dayjs fix in vite config: add optimizeDeps.include and resolve.alias for dayjs to fix 'default export not found' SyntaxError in Vite dev server - Update .npmrc: remove shamefully-hoist, add blockExoticSubdeps and trustPolicy=no-downgrade - Add pnpm-workspace.yaml exception to .gitignore
Add dayjs as direct dependency so it is hoisted to node_modules root and resolvable via require.resolve() in vite config. Use createRequire from node:module with an alias pointing to dayjs/dayjs.min.js to fix the 'default export not found' SyntaxError from Vite's ESM/CJS interop.
pnpm-workspace.yaml contains the overrides configuration that must match the lockfile. Without it in the build context pnpm reports ERR_PNPM_LOCKFILE_CONFIG_MISMATCH and fails --frozen-lockfile install.
Without shamefully-hoist, pnpm does not hoist transitive deps in Docker. vitepress-plugin-mermaid registers these packages in optimizeDeps.include but they are not resolvable at the project root, causing a blank page: - @braintree/sanitize-url - cytoscape - cytoscape-cose-bilkent Adding them as direct deps ensures they are hoisted alongside debug and dayjs.
…ufficient Vite's pre-bundling via optimizeDeps.include handles the CJS/ESM interop for dayjs automatically. The explicit alias to dayjs.min.js and createRequire are not needed and caused a SyntaxError in the browser.
VelmiraS
force-pushed
the
fix/remove-unused-deps-supply-chain
branch
from
87b7ec0 to
d0aee15
Compare
gardener-prow
Bot
removed
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.npmrc:
- Around line 1-4: The keys minimum-release-age, blockExoticSubdeps and
trustPolicy in .npmrc are ignored by pnpm v11, so move those supply-chain
hardening settings into pnpm-workspace.yaml and replace any use of the removed
onlyBuiltDependencies behavior with the new allowBuilds map; specifically add
top-level keys minimumReleaseAge, blockExoticSubdeps and trustPolicy in
pnpm-workspace.yaml (note the camelCase names) and create an allowBuilds mapping
of package patterns to booleans for packages that are permitted to run builds
(e.g., set esbuild: true), removing those keys from .npmrc.
In `@Makefile`:
- Around line 201-203: The Makefile installs the Windows Vale binary as
bin/vale.exe but later invokes bin/vale (causing Windows CI to fail); update the
install step or the runner invocation so both platforms use the same executable
name — e.g., after extracting vale.exe in the installation recipe create a
platform-aware alias or copy/rename to bin/vale (or detect OS in the vale-run
target and call bin/vale.exe on Windows); modify the installation lines that
reference bin/vale.exe and the vale-run invocation to consistently use the same
name (symbols to change: the install recipe that extracts vale.exe and the
vale-run target that calls bin/vale).
- Around line 190-199: The Makefile currently pipes remote archives from curl
directly into tar for the Vale releases (see the curl + tar lines for
vale_3.14.2_macOS_arm64.tar.gz, vale_3.14.2_macOS_64-bit.tar.gz,
vale_3.14.2_Linux_arm64.tar.gz and vale_3.14.2_Linux_64-bit.tar.gz); change this
to first download the archive and its checksum/signature to a temporary file,
verify integrity (e.g., compare sha256sum against the vendor-provided .sha256 or
verify a GPG .asc signature using a pinned public key), fail the build if
verification fails, then extract the verified archive into bin; ensure the
Makefile uses the same unique curl targets shown in those lines and cleans up
temp files on failure/success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 246a09ae-2950-4fec-8180-f28a33f1d687
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.dockerignore.gitignore.npmrc.vitepress/config.mts.vitepress/theme/blog-sidebar.tsDockerfileMakefilepackage.jsonpnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (2)
- pnpm-workspace.yaml
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- .vitepress/theme/blog-sidebar.ts
- Dockerfile
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.npmrc:
- Around line 1-4: The keys minimum-release-age, blockExoticSubdeps and
trustPolicy in .npmrc are ignored by pnpm v11, so move those supply-chain
hardening settings into pnpm-workspace.yaml and replace any use of the removed
onlyBuiltDependencies behavior with the new allowBuilds map; specifically add
top-level keys minimumReleaseAge, blockExoticSubdeps and trustPolicy in
pnpm-workspace.yaml (note the camelCase names) and create an allowBuilds mapping
of package patterns to booleans for packages that are permitted to run builds
(e.g., set esbuild: true), removing those keys from .npmrc.
In `@Makefile`:
- Around line 201-203: The Makefile installs the Windows Vale binary as
bin/vale.exe but later invokes bin/vale (causing Windows CI to fail); update the
install step or the runner invocation so both platforms use the same executable
name — e.g., after extracting vale.exe in the installation recipe create a
platform-aware alias or copy/rename to bin/vale (or detect OS in the vale-run
target and call bin/vale.exe on Windows); modify the installation lines that
reference bin/vale.exe and the vale-run invocation to consistently use the same
name (symbols to change: the install recipe that extracts vale.exe and the
vale-run target that calls bin/vale).
- Around line 190-199: The Makefile currently pipes remote archives from curl
directly into tar for the Vale releases (see the curl + tar lines for
vale_3.14.2_macOS_arm64.tar.gz, vale_3.14.2_macOS_64-bit.tar.gz,
vale_3.14.2_Linux_arm64.tar.gz and vale_3.14.2_Linux_64-bit.tar.gz); change this
to first download the archive and its checksum/signature to a temporary file,
verify integrity (e.g., compare sha256sum against the vendor-provided .sha256 or
verify a GPG .asc signature using a pinned public key), fail the build if
verification fails, then extract the verified archive into bin; ensure the
Makefile uses the same unique curl targets shown in those lines and cleans up
temp files on failure/success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 246a09ae-2950-4fec-8180-f28a33f1d687
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.dockerignore.gitignore.npmrc.vitepress/config.mts.vitepress/theme/blog-sidebar.tsDockerfileMakefilepackage.jsonpnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (2)
- pnpm-workspace.yaml
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- .vitepress/theme/blog-sidebar.ts
- Dockerfile
🛑 Comments failed to post (3)
.npmrc (1)
1-4:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick win🧩 Analysis chain
🌐 Web query:
For pnpm v11, which .npmrc keys are valid for supply-chain hardening? Specifically, istrustPolicysupported, and what is the correct syntax foronlyBuiltDependenciesin.npmrc?💡 Result:
In pnpm v11,.npmrc files are strictly reserved for registry, authentication, and specific network settings [1][2][3]. All other configuration settings, including those related to supply-chain hardening, must be defined in pnpm-workspace.yaml (for project-specific settings) or the global ~/.config/pnpm/config.yaml file [1][2][4]. Regarding your specific questions: 1. trustPolicy: This setting is supported for supply-chain hardening but is not a valid.npmrc key in v11 [1][2][5]. You must configure it in your pnpm-workspace.yaml file [2][5]. When set to no-downgrade, it prevents the installation of a package if its trust level (e.g., presence of signatures or provenance) has decreased compared to previous releases [6][7][8]. 2. onlyBuiltDependencies: This setting has been removed in pnpm v11 [9]. It has been replaced by the allowBuilds configuration, which must also be defined in pnpm-workspace.yaml (or the global config) rather than.npmrc [1][9]. The allowBuilds configuration allows you to define build permissions using a structured YAML format [9]. Summary of Configuration Changes -.npmrc: Only for authentication (tokens, passwords, tokenHelpers), registry URLs, and network settings (e.g., strict-ssl, proxies) [3]. - pnpm-workspace.yaml: The required location for pnpm-specific hardening settings like trustPolicy and allowBuilds [1][4][9]. To migrate from the old onlyBuiltDependencies setting, you should remove it from your.npmrc and implement the new allowBuilds structure in your pnpm-workspace.yaml [1][9].
Citations:
- 1: https://pnpm.io/blog/releases/11.0
- 2: https://github.com/pnpm/pnpm/releases/tag/v11.0.0
- 3: pnpm/pnpm#11189
- 4: https://pnpm.io/migration
- 5: https://pnpm.io/settings
- 6: https://pnpm.io/supply-chain-security
- 7: https://pnpm.io/10.x/settings
- 8: https://pnpm.io/next/supply-chain-security
- 9: https://github.com/pnpm/pnpm/releases/tag/v11.0.0-rc.0
🌐 Web query:
pnpm v11 allowBuilds configuration syntax pnpm-workspace.yaml💡 Result:
In pnpm v11, the allowBuilds configuration is defined in the pnpm-workspace.yaml file to manage which package scripts are allowed to execute during installation [1][2]. This setting replaces several legacy configurations, including onlyBuiltDependencies, neverBuiltDependencies, ignoredBuiltDependencies, and ignoreDepScripts [1][3][2]. Syntax The allowBuilds setting is a map where keys are package name patterns and values are booleans (true to allow, false to deny) [1][2]. Example configuration in pnpm-workspace.yaml: allowBuilds: esbuild: true core-js: false "nx@21.6.4 || 21.6.5": true Key Details - Behavior: Dependencies not listed in allowBuilds are disallowed by default [2]. When an unreviewed package with build scripts is encountered, pnpm will typically print an error or warning depending on your strictDepBuilds setting [2]. - Automatic Updates: During installation, if pnpm encounters dependencies with ignored builds that are not yet in allowBuilds, it automatically adds them to pnpm-workspace.yaml with a placeholder value, allowing you to manually toggle them to true or false [4][2]. - Management: You can manage these settings interactively using the pnpm approve-builds command, which updates the pnpm-workspace.yaml file automatically [4]. - Migration: If you are upgrading to v11, these legacy settings from.npmrc or package.json should be removed and consolidated into the allowBuilds map within pnpm-workspace.yaml [1][3].
Citations:
- 1: https://pnpm.io/blog/releases/11.0
- 2: https://pnpm.io/settings
- 3: https://github.com/pnpm/codemod/blob/main/v11/README.md
- 4: https://pnpm.io/cli/approve-builds
🌐 Web query:
pnpm v11 trustPolicy no-downgrade location syntax pnpm-workspace.yaml💡 Result:
To configure the trust policy in pnpm (including v11), you add the trustPolicy setting to your pnpm-workspace.yaml file [1][2][3]. The correct syntax to enable this policy is [1][4]: trustPolicy: no-downgrade When set to no-downgrade, pnpm prevents the installation of a package if its trust level (based on publish date and evidence such as trusted publisher status or provenance) has decreased relative to earlier versions [1][5]. Additionally, you can use the following related settings in pnpm-workspace.yaml to manage trust behavior [1][6]: - trustPolicyExclude: A list of package selectors to ignore during trust policy checks (e.g., trustPolicyExclude: ['package@version']) [1][4]. - trustPolicyIgnoreAfter: A number (in minutes) to ignore trust checks for packages published longer than this duration ago [6][4]. - trustLockfile: A boolean setting (default: false) that, when set to true, allows pnpm to skip the supply-chain verification pass on already-trusted entries in the lockfile [2][3]. These settings collectively help mitigate supply chain attacks by enforcing consistent security evidence requirements across package updates [5][7].
Citations:
- 1: https://pnpm.io/settings
- 2: https://pnpm.io/blog/releases/11.3
- 3: https://github.com/pnpm/pnpm/releases/tag/v11.3.0
- 4: https://github.com/pnpm/pnpm.io/blob/main/versioned_docs/version-10.x/settings.md
- 5: https://pnpm.io/supply-chain-security
- 6: https://pnpm.io/10.x/settings
- 7: https://pnpm.io/next/supply-chain-security
🌐 Web query:
pnpm v11 which file should define supply-chain security settings vs .npmrc settings list💡 Result:
In pnpm v11, configuration settings are strictly separated into two categories based on their purpose, and supply-chain security settings must be defined in YAML configuration files rather than.npmrc [1][2]. 1. Where to define supply-chain security settings Supply-chain security settings must be defined in your YAML configuration files [1][2]. The primary location for project-level settings is the pnpm-workspace.yaml file located at the root of your workspace [2][3]. For global settings that apply across all projects on your machine, use the global config.yaml file (typically located at ~/.config/pnpm/config.yaml on Linux/macOS) [2][3]. Examples of supply-chain security settings that belong in these YAML files include: - minimumReleaseAge (defaults to 1440 minutes/1 day in v11) [4][1] - blockExoticSubdeps (defaults to true in v11) [4][1] - trustPolicy [4] - trustPolicyExclude [4] - trustPolicyIgnoreAfter [4] - trustLockfile [5][6] - allowBuilds (used to manage which packages may run build scripts) [1][7] 2. Role of.npmrc In pnpm v11,.npmrc files (and the global rc file) are reserved exclusively for registry and authentication settings [1][2]. Examples include registry URLs, authentication tokens (_authToken), and user credentials [1][8]. pnpm no longer reads general configuration settings from.npmrc [1][9]. If you are migrating from an older version, you must move all non-auth/registry settings out of.npmrc and into pnpm-workspace.yaml or config.yaml [1][9]. Using the wrong file for these settings will result in them being ignored, as pnpm v11 strictly enforces this separation [1][2].
Citations:
- 1: https://pnpm.io/blog/releases/11.0
- 2: https://pnpm.io/configuring
- 3: https://pnpm.io/cli/config
- 4: https://pnpm.io/supply-chain-security
- 5: https://pnpm.io/settings
- 6: https://newreleases.io/project/github/pnpm/pnpm/release/v11.3.0
- 7: https://pnpm.io/next/supply-chain-security
- 8: https://pnpm.io/npmrc
- 9: https://pnpm.io/migration
Supply-chain hardening in
.npmrcwon’t take effect under pnpm v11.
- pnpm v11 only uses
.npmrcfor registry/auth/network settings;minimum-release-age,blockExoticSubdeps, andtrustPolicyin.npmrc(Lines 1-4) are ignored, so the hardening is not enforced.onlyBuiltDependenciesis removed in pnpm v11; replace it withallowBuildsinpnpm-workspace.yaml(YAML map of package patterns → booleans), e.g.:# pnpm-workspace.yaml minimumReleaseAge: 10080 blockExoticSubdeps: true trustPolicy: no-downgrade allowBuilds: esbuild: true sharp: true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.npmrc around lines 1 - 4, The keys minimum-release-age, blockExoticSubdeps and trustPolicy in .npmrc are ignored by pnpm v11, so move those supply-chain hardening settings into pnpm-workspace.yaml and replace any use of the removed onlyBuiltDependencies behavior with the new allowBuilds map; specifically add top-level keys minimumReleaseAge, blockExoticSubdeps and trustPolicy in pnpm-workspace.yaml (note the camelCase names) and create an allowBuilds mapping of package patterns to booleans for packages that are permitted to run builds (e.g., set esbuild: true), removing those keys from .npmrc.Makefile (2)
190-199:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify downloaded Vale binaries before extraction.
Line 190, Line 192, Line 196, Line 198, and Line 201 download and extract executable artifacts without integrity verification. That is a supply-chain gap in a PR explicitly hardening dependency provenance.
Suggested hardening pattern
+VALE_VERSION := 3.14.2 +# Fill with official release checksums from vale-cli release assets +VALE_SHA256_LINUX_X64 := <expected_sha256> + .PHONY: vale-install vale-install: ## Install Vale binary if not already present `@if` [ -x bin/vale ]; then \ echo "Vale is already installed: $$(bin/vale --version)"; \ else \ echo "Installing Vale..."; \ mkdir -p bin; \ if [ "$$(uname)" = "Linux" ]; then \ if [ "$$(uname -m)" = "aarch64" ] || [ "$$(uname -m)" = "arm64" ]; then \ - curl -sL https://github.com/vale-cli/vale/releases/download/v3.14.2/vale_3.14.2_Linux_arm64.tar.gz | tar -xz -C bin vale; \ + curl -sL -o /tmp/vale.tar.gz https://github.com/vale-cli/vale/releases/download/v$(VALE_VERSION)/vale_$(VALE_VERSION)_Linux_arm64.tar.gz; \ + echo "$(VALE_SHA256_LINUX_ARM64) /tmp/vale.tar.gz" | sha256sum -c -; \ + tar -xzf /tmp/vale.tar.gz -C bin vale; \ else \ - curl -sL https://github.com/vale-cli/vale/releases/download/v3.14.2/vale_3.14.2_Linux_64-bit.tar.gz | tar -xz -C bin vale; \ + curl -sL -o /tmp/vale.tar.gz https://github.com/vale-cli/vale/releases/download/v$(VALE_VERSION)/vale_$(VALE_VERSION)_Linux_64-bit.tar.gz; \ + echo "$(VALE_SHA256_LINUX_X64) /tmp/vale.tar.gz" | sha256sum -c -; \ + tar -xzf /tmp/vale.tar.gz -C bin vale; \ fi; \ fi; \ fiAlso applies to: 201-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 190 - 199, The Makefile currently pipes remote archives from curl directly into tar for the Vale releases (see the curl + tar lines for vale_3.14.2_macOS_arm64.tar.gz, vale_3.14.2_macOS_64-bit.tar.gz, vale_3.14.2_Linux_arm64.tar.gz and vale_3.14.2_Linux_64-bit.tar.gz); change this to first download the archive and its checksum/signature to a temporary file, verify integrity (e.g., compare sha256sum against the vendor-provided .sha256 or verify a GPG .asc signature using a pinned public key), fail the build if verification fails, then extract the verified archive into bin; ensure the Makefile uses the same unique curl targets shown in those lines and cleans up temp files on failure/success.
201-203:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the correct Vale executable name on Windows.
Line 201–203 installs
bin/vale.exe, but Line 217 always runsbin/vale. On Windows runners this can breakvale-run.Cross-platform fix
+.PHONY: vale-run vale-run: ## Lint changed website markdown files with Vale + `@VALE_BIN`=bin/vale; \ + if echo "$$(uname -s)" | grep -qi "mingw\|cygwin\|msys"; then VALE_BIN=bin/vale.exe; fi; \ `@CHANGED`=$$(git diff --name-only --diff-filter=d HEAD -- 'website/**/*.md' | \ grep -v '^website/about/legal-disclosure\.md' | \ grep -v '^website/archived/'); \ if [ -n "$$CHANGED" ]; then \ - bin/vale $$CHANGED; \ + $$VALE_BIN $$CHANGED; \ else \ echo "No changed .md files to lint."; \ fiAlso applies to: 217-217
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 201 - 203, The Makefile installs the Windows Vale binary as bin/vale.exe but later invokes bin/vale (causing Windows CI to fail); update the install step or the runner invocation so both platforms use the same executable name — e.g., after extracting vale.exe in the installation recipe create a platform-aware alias or copy/rename to bin/vale (or detect OS in the vale-run target and call bin/vale.exe on Windows); modify the installation lines that reference bin/vale.exe and the vale-run invocation to consistently use the same name (symbols to change: the install recipe that extracts vale.exe and the vale-run target that calls bin/vale).
|
About the Vale targets (comments on Makefile lines 190–217) The The missing checksum verification and the About the .npmrc settings (comment on lines 1–4) You are correct that We have already moved I suggest opening a separate issue |
2 participants
/kind task
What this PR does / why we need it:
Hardens the supply chain by migrating from npm to pnpm and removing unused dependencies.
packageManager: pnpm@10.11.0,corepack enablein Dockerfile)autoprefixerandlodash-es(replaces_.orderBy()inblog-sidebar.tswith nativeArray.sort())^or~ranges).npmrc:frozen-lockfile=true,minimum-release-age=10080(7-day cooldown),onlyBuiltDependenciesallowlist for postinstall scripts (esbuild,sharp)pnpm install --frozen-lockfile)shamefully-hoist=truefor VitePress compatibility (required for Vite's module resolver to access transitive deps)package-lock.jsonwithpnpm-lock.yamlWhich issue(s) this PR fixes:
Special notes for your reviewer:
.npmrcandpnpm-lock.yamlmust both be present in the Docker build context —.dockerignorehas been updated accordinglytrust-policy=no-downgradewas intentionally not added — it is not a valid pnpm config key and would be silently ignoredshamefully-hoist=trueis explicitly recommended by the VitePress docs for pnpm users; without itvitepress-plugin-mermaid's transitive deps are not resolvable by Vite, resulting in a blank pagepnpm installoutside of Docker should be updated to usepnpm install --frozen-lockfileSummary by CodeRabbit
Chores
Refactor