Skip to content

Add .npmrc with security hardening settings#462

Merged
esezen merged 2 commits into
masterfrom
security/add-npmrc
Jun 10, 2026
Merged

Add .npmrc with security hardening settings#462
esezen merged 2 commits into
masterfrom
security/add-npmrc

Conversation

@danmcc-admin

Copy link
Copy Markdown
Contributor

Summary

  • Adds .npmrc with ignore-scripts=true and save-exact=true
  • ignore-scripts=true prevents arbitrary code execution during npm install (supply chain defense)
  • save-exact=true pins exact dependency versions for reproducible builds

If a package requires install scripts

Add a per-package allowlist in .npmrc:

lifecycle-script-allowed[]=sharp
lifecycle-script-allowed[]=esbuild

Or run one-off with:

npm install --ignore-scripts=false <package-name>

Context

Part of an org-wide security hardening initiative for all npm-based repositories.

Security hardening: ignore-scripts prevents malicious install scripts
from running during npm install, and save-exact pins dependency versions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 10, 2026 01:47
@danmcc-admin danmcc-admin requested a review from a team as a code owner June 10, 2026 01:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a repository-level .npmrc to harden npm installs by disabling lifecycle scripts and pinning exact dependency versions, aligning with an org-wide supply-chain security initiative.

Changes:

  • Add .npmrc with ignore-scripts=true to prevent lifecycle script execution during installs.
  • Add .npmrc with save-exact=true to pin exact dependency versions in package.json.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .npmrc
constructor-claude-bedrock[bot]

This comment was marked as outdated.

@constructor-claude-bedrock constructor-claude-bedrock Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This PR adds a .npmrc with supply-chain hardening settings and documents the resulting developer workflow change in the README; the intent is sound, but there is one important functional concern and a couple of minor issues worth addressing.

Inline comments: 4 discussions added

Overall Assessment: ⚠️ Needs Work

Comment thread .npmrc
@@ -0,0 +1,2 @@
ignore-scripts=true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important Issue: ignore-scripts=true blocks all lifecycle scripts for all packages, including those that require them to function correctly after a fresh install. The most relevant example in this repo is esbuild (a devDependency declared in package.json), which uses a postinstall script to download the correct platform binary. With ignore-scripts=true, a fresh npm install on a new machine will produce a non-functional esbuild binary, and npm run bundle will fail silently or with a confusing error.

The PR description acknowledges this with the lifecycle-script-allowed[] workaround, but that syntax (lifecycle-script-allowed) is not a real npm config key — it does not exist in any npm version. The valid approaches are:

  1. Use npm install --ignore-scripts=false esbuild after initial install (manual, error-prone for new contributors)
  2. Move esbuild to use a wrapper package that doesn't rely on install scripts (e.g., esbuild-wasm)
  3. Use a .npmrc per-package override via //registry.npmjs.org/:_authToken style or a tool like pnpm that has a proper allowlist
  4. Accept the tradeoff and document it clearly, then require contributors to re-run npm install --ignore-scripts=false esbuild when needed

At minimum, the README note should be expanded to mention that esbuild may need manual install if it's not already cached in node_modules.

Comment thread .npmrc
@@ -0,0 +1,2 @@
ignore-scripts=true
save-exact=true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: save-exact=true pins exact versions when running npm install <pkg> but has no effect on the already-committed package.json — existing entries like "esbuild": "^0.25.0" keep their caret ranges. This setting only changes behavior for future npm install <new-package> commands run by developers.

If the goal is truly reproducible builds, the existing package-lock.json already provides that guarantee — save-exact=true is complementary but not sufficient on its own to harden the existing entries. Consider whether the intent is:

  • Enforcing exact pins for future additions (this setting does that — acceptable, just document it)
  • Pinning all current entries (would require updating package.json ranges to exact versions — a separate, larger change)

This is a low-risk setting but the distinction should be made clear in the PR description or a follow-up comment.

Comment thread README.md
## Development / npm commands

```bash
npm run prepare # one-time after cloning: install husky git hooks (pre-push lint)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The comment says "one-time after cloning" but npm run prepare should also be re-run after npm ci in a clean CI-like environment (since ignore-scripts=true will suppress it). Consider rewording to make it clear this is required any time node_modules is re-created from scratch:

npm run prepare       # run once after npm install (sets up husky git hooks)

Also, since this is a client-side browser library consumed by downstream projects, it is worth noting that the .npmrc will affect consumers who clone the repo, but ignore-scripts=true does not propagate to consumers who install the published npm package — so there is no downstream impact for library users.

Comment thread README.md
npm run docs # output documentation to `./docs` directory
```

> **Note:** `.npmrc` sets `ignore-scripts=true` for supply-chain safety, so lifecycle scripts (including husky's hook installation) do not run automatically on `npm install`. Run `npm run prepare` once after cloning to enable the `pre-push` lint hook.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important Issue: This note only mentions husky, but omits the more impactful case: esbuild's binary setup also relies on a postinstall script. A new contributor following these instructions exactly (npm install then npm run prepare) will have a broken esbuild and will hit a confusing failure when running npm run bundle or any build step.

The note should either:

  1. Explicitly call out that esbuild requires npm install --ignore-scripts=false esbuild after the initial install, OR
  2. If esbuild happens to be bundled without its native binary in the node_modules already committed (which would be unusual), clarify that.

Recommended expanded note:

> **Note:** `.npmrc` sets `ignore-scripts=true` for supply-chain safety. After `npm install`:
> 1. Run `npm run prepare` once to install the husky `pre-push` lint hook.
> 2. Run `npm install --ignore-scripts=false esbuild` if `npm run bundle` fails (esbuild requires its postinstall script to download the platform binary).

@esezen esezen merged commit 5fb5a8e into master Jun 10, 2026
15 checks passed
@esezen esezen deleted the security/add-npmrc branch June 10, 2026 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants