From 5ce17f66b79b84b2df539cb20b584d950eec600b Mon Sep 17 00:00:00 2001 From: "hivecommons-hive[bot]" Date: Sat, 26 Sep 2026 11:45:13 -0400 Subject: [PATCH] fix: install from the lockfile on the documented setup path Every CI workflow installs with `npm ci`, but README.md, CONTRIBUTING.md, AGENTS.md and the devcontainer's postCreateCommand all told a human to run `npm install`. That command does not read package-lock.json as an instruction: it re-resolves the caret ranges in package.json against the registry, runs the lifecycle scripts of whatever it resolved, and rewrites the lockfile in the contributor's tree. So the documented path installed and executed dependency code that was never reviewed or pinned, skipped the lockfile's integrity hashes, and let a dependency upgrade drift into an unrelated PR. Point all four at `npm ci` and add a regression test to tests/dev-environment.test.mjs that rejects a bare `npm install` in a developer doc or a devcontainer lifecycle command. `npm install ` is still the way to add a dependency, so only the bare form is rejected. Closes #703 Signed-off-by: hivecommons-hive[bot] --- .devcontainer/devcontainer.json | 2 +- AGENTS.md | 4 ++- CONTRIBUTING.md | 6 +++- README.md | 8 +++++- tests/dev-environment.test.mjs | 49 +++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 4 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 7717f9af..9317c15f 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -26,7 +26,7 @@ }, "forwardPorts": [3000], "containerUser": "vscode", - "postCreateCommand": "npm install", + "postCreateCommand": "npm ci", "waitFor": "postCreateCommand", // otherwise automated jest tests fail "features": { "node": { diff --git a/AGENTS.md b/AGENTS.md index 56401424..bd2454d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,9 @@ fail the PR. ## Build & Test -- **Install**: `npm install` +- **Install**: `npm ci` — installs the exact versions in `package-lock.json`, as + CI does. Use `npm install ` only to add or upgrade a dependency on + purpose; it re-resolves the semver ranges and rewrites the lockfile. - **Start Dev Server**: `npm run docus:start` - **Build**: `npm run build` - **Unit Tests**: `npm run test:unit` — required "Validate repository" CI check diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 32326e01..e17ccd6f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,10 +8,14 @@ This document covers the development workflow and the data contribution model. Prerequisites: Node.js 22+ (LTS recommended) and npm. ```bash -npm install +npm ci npm run docus:start ``` +`npm ci` installs the exact, integrity-checked versions in `package-lock.json`, +matching CI. Reach for `npm install ` only to add or upgrade a +dependency, and commit the resulting lockfile change deliberately. + The dev server runs at `http://localhost:3000`. In the devcontainer: ```bash diff --git a/README.md b/README.md index 91528030..77b6c853 100644 --- a/README.md +++ b/README.md @@ -34,9 +34,15 @@ Ensure you have the following installed: ### Install dependencies ```bash -npm install +npm ci ``` +`npm ci` installs exactly the versions recorded in `package-lock.json` and +verifies their integrity hashes, which is what CI runs. Use +`npm install ` only when you intend to add or upgrade a dependency; +that re-resolves the ranges in `package.json` and rewrites the lockfile, so the +change belongs in its own reviewed commit. + ### Run the site ```bash diff --git a/tests/dev-environment.test.mjs b/tests/dev-environment.test.mjs index 57443cc0..2c4fbfef 100644 --- a/tests/dev-environment.test.mjs +++ b/tests/dev-environment.test.mjs @@ -177,6 +177,55 @@ test('every npm script invoked by devcontainer lifecycle commands exists', () => ); }); +// CI installs with `npm ci`, the only npm command that treats +// package-lock.json as an instruction: exact versions, integrity hashes +// verified, lockfile left alone. A bare `npm install` re-resolves the caret +// ranges in package.json against the registry, runs the lifecycle scripts of +// whatever it picked, and rewrites the lockfile in the contributor's tree — so +// a doc that prescribes it hands every contributor an unreviewed dependency +// graph and lets an upgrade ride into an unrelated PR. `npm install ` is +// still the right way to add a dependency, so only the bare form is rejected. +const BARE_NPM_INSTALL = /\bnpm install\s*(?=$|[\n`'"&|;])/; + +test('no developer doc or devcontainer command prescribes a bare npm install', () => { + const offenders = []; + + for (const doc of ['README.md', 'CONTRIBUTING.md', 'AGENTS.md']) { + const lines = readFileSync(join(root, doc), 'utf8').split('\n'); + for (const [index, line] of lines.entries()) { + if (BARE_NPM_INSTALL.test(line)) offenders.push(`${doc}:${index + 1}`); + } + } + + const devcontainer = JSON.parse(stripJsonComments(devcontainerRaw)); + for (const key of [ + 'initializeCommand', + 'onCreateCommand', + 'updateContentCommand', + 'postCreateCommand', + 'postStartCommand', + 'postAttachCommand', + ]) { + const value = devcontainer[key]; + const commands = + typeof value === 'string' + ? [value] + : Array.isArray(value) + ? [value.join(' ')] + : value && typeof value === 'object' + ? Object.values(value).map((entry) => String(entry)) + : []; + if (commands.some((command) => BARE_NPM_INSTALL.test(command))) + offenders.push(`devcontainer.${key}`); + } + + assert.deepEqual( + offenders, + [], + `these prescribe a bare \`npm install\`, which ignores package-lock.json; use \`npm ci\`: ${offenders.join(', ')}`, + ); +}); + // The dev port the devcontainer forwards has to be the one `just serve` // actually opens, or the recipe starts a server nobody outside the container // can reach.