Add liquidjs override and security check script enforcing >=10.25.0#2
Add liquidjs override and security check script enforcing >=10.25.0#2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enforces a minimum safe liquidjs version for the Eleventy site by pinning the dependency via npm overrides and adding a scriptable version check that can be run in CI/local workflows.
Changes:
- Added a Node script (
scripts/check-liquidjs-version.js) that fails if the installedliquidjsversion is below10.25.0. - Added
test:securitynpm script to run the version check. - Added an npm
overridesentry forliquidjsand updated the lockfile toliquidjs@10.25.0.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
eleventy/scripts/check-liquidjs-version.js |
New security check script to validate the installed liquidjs version meets a minimum. |
eleventy/package.json |
Adds test:security script and pins liquidjs via overrides. |
eleventy/package-lock.json |
Updates resolved liquidjs package to 10.25.0 (and its engine metadata). |
Files not reviewed (1)
- eleventy/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const pkg = require('../node_modules/liquidjs/package.json'); | ||
| const version = pkg.version; | ||
| const min = process.env.MIN_LIQUIDJS_VERSION || '10.25.0'; | ||
|
|
||
| function parse(v) { | ||
| return v.split('.').map((n) => Number(n)); | ||
| } | ||
|
|
||
| function gte(a, b) { | ||
| for (let i = 0; i < Math.max(a.length, b.length); i++) { | ||
| const av = a[i] || 0; | ||
| const bv = b[i] || 0; | ||
| if (av > bv) return true; | ||
| if (av < bv) return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| if (!gte(parse(version), parse(min))) { |
There was a problem hiding this comment.
The script requires LiquidJS' package.json via a hard-coded ../node_modules path and then compares versions using a custom parser. This is brittle across install layouts (e.g., nested deps, different package managers) and the parser can mis-handle valid semver strings (pre-releases/build metadata) producing NaN and incorrect comparisons. Prefer resolving the package via Node resolution (e.g., require.resolve) and using the existing semver dependency to do a correct >= check, with a clear error if resolution fails.
| const pkg = require('../node_modules/liquidjs/package.json'); | |
| const version = pkg.version; | |
| const min = process.env.MIN_LIQUIDJS_VERSION || '10.25.0'; | |
| function parse(v) { | |
| return v.split('.').map((n) => Number(n)); | |
| } | |
| function gte(a, b) { | |
| for (let i = 0; i < Math.max(a.length, b.length); i++) { | |
| const av = a[i] || 0; | |
| const bv = b[i] || 0; | |
| if (av > bv) return true; | |
| if (av < bv) return false; | |
| } | |
| return true; | |
| } | |
| if (!gte(parse(version), parse(min))) { | |
| const semver = require('semver'); | |
| let pkg; | |
| try { | |
| const liquidPkgPath = require.resolve('liquidjs/package.json'); | |
| pkg = require(liquidPkgPath); | |
| } catch (err) { | |
| console.error('Unable to resolve liquidjs/package.json. Ensure liquidjs is installed as a dependency.'); | |
| process.exit(1); | |
| } | |
| const version = pkg && pkg.version; | |
| const min = process.env.MIN_LIQUIDJS_VERSION || '10.25.0'; | |
| if (!version || !semver.valid(version)) { | |
| console.error(`Installed liquidjs version "${version}" is not a valid semantic version.`); | |
| process.exit(1); | |
| } | |
| if (!semver.valid(min)) { | |
| console.error(`Configured minimum liquidjs version "${min}" is not a valid semantic version.`); | |
| process.exit(1); | |
| } | |
| if (!semver.gte(version, min)) { |
| const min = process.env.MIN_LIQUIDJS_VERSION || '10.25.0'; | ||
|
|
There was a problem hiding this comment.
The minimum version is duplicated: it's hard-coded here as '10.25.0' while package.json also pins the override to ^10.25.0. This can drift over time (e.g., override bumped but the check script still enforces the old value). Consider sourcing the default minimum from package.json overrides (or a single shared constant) and only letting MIN_LIQUIDJS_VERSION override that when explicitly set.
| const min = process.env.MIN_LIQUIDJS_VERSION || '10.25.0'; | |
| const rootPkg = require('../package.json'); | |
| function getDefaultMinVersionFromOverrides(packageJson) { | |
| const overrides = packageJson && packageJson.overrides; | |
| if (!overrides) { | |
| return '10.25.0'; | |
| } | |
| const overrideEntry = overrides.liquidjs || overrides['liquidjs']; | |
| if (typeof overrideEntry !== 'string' || overrideEntry.length === 0) { | |
| return '10.25.0'; | |
| } | |
| // Strip leading non-digit characters (e.g., "^10.25.0" -> "10.25.0") | |
| const numericVersion = overrideEntry.replace(/^[^\d]*/, ''); | |
| return numericVersion || '10.25.0'; | |
| } | |
| const min = process.env.MIN_LIQUIDJS_VERSION || getDefaultMinVersionFromOverrides(rootPkg); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed81ac388f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "start": "npx eleventy --serve", | ||
| "build": "npx eleventy" | ||
| "build": "npx eleventy", | ||
| "test:security": "node scripts/check-liquidjs-version.js" |
There was a problem hiding this comment.
Adding test:security here makes the repository fail its new check out of the box, because the commit does not also update the tracked vendored package at eleventy/node_modules/liquidjs/package.json (still 10.17.0), so npm run test:security immediately exits non-zero against the committed tree. Since this repo tracks node_modules, the change leaves the vulnerable version in-tree while enforcing >=10.25.0, which breaks CI/local validation unless everyone reinstalls dependencies first.
Useful? React with 👍 / 👎.
Motivation
liquidjsrelease by pinning and checking the installed version to avoid incompatible or insecure older releases.Description
scripts/check-liquidjs-version.js, a small Node script that parses the installedliquidjsversion and fails if it is below10.25.0.test:securitynpm script that runs the version check withnode scripts/check-liquidjs-version.js.overridesentry forliquidjsset to^10.25.0ineleventy/package.jsonand updatedeleventy/package-lock.jsonto reflectliquidjs@10.25.0(which also updates thenodeengine requirement to>=16).package.jsonscript formatting adjusted (buildscript trailing comma) to accommodate the new script and override entries.Testing
npm run test:securityto perform the new version check in CI or locally.Codex Task