Skip to content

Feature: Improve developer security#145

Open
infeo wants to merge 14 commits intodevelopfrom
feature/improve-security
Open

Feature: Improve developer security#145
infeo wants to merge 14 commits intodevelopfrom
feature/improve-security

Conversation

@infeo
Copy link
Copy Markdown
Member

@infeo infeo commented May 5, 2026

This PR improves the development security.

It follows suggestions of https://github.com/lirantal/npm-security-best-practices.

The main change is to use pnpm instead of npm for the following reasons:

  • disabling post-install scripts
  • disabling of non-registry transitive dependencies
  • cooldown period for dependency updates
  • pnpm-lock.yaml records integrity per package
  • non-flat node_modules layout prevents phantom dependencies, so a
    compromised transitive cannot be require()-d directly by app code

Changes:

  • pnpm-lock.yaml replacing package-lock.json
  • pnpm-workspace.yaml config added
  • setting packageManager field in package file for corepack setup
  • pre-build check for strange direct dependencies (git or http sources)
  • pre-execution check if npm is still used

Additional changes:

  • add dependabot file
  • update README/CONTRIBUTE

Remark: This PR is based upon #144

@infeo infeo changed the base branch from develop to feature/docusaurus-3.10.0 May 5, 2026 08:56
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 701bcec6-c4c4-4924-954a-9875676dec35

📥 Commits

Reviewing files that changed from the base of the PR and between 486d5c5 and 26caa17.

📒 Files selected for processing (2)
  • README.md
  • scripts/check-lockfile.sh
✅ Files skipped from review due to trivial changes (1)
  • README.md

Walkthrough

This pull request migrates repository tooling and docs from npm to pnpm: docs and README now instruct corepack enable and pnpm commands; package.json pins pnpm@10.33.2 and adds check-pm.js enforcement; CI (.github/workflows/deploy.yml) switches to pnpm with lockfile validation and pnpm caching; a pnpm-workspace.yaml introduces strict dependency/build controls and minimumReleaseAge; a lockfile guard script (scripts/check-lockfile.sh) and Dependabot config (.github/dependabot.yml) were added; .gitignore ignores package-lock.json.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Feature: Improve developer security' is vague and does not clearly describe the specific primary change (migration from npm to pnpm and associated security improvements). Consider a more specific title that highlights the main change, such as 'Migrate from npm to pnpm for improved security' or 'Switch package manager to pnpm and add security hardening'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the security motivations, main change (npm to pnpm migration), concrete implementation details, and references the npm-security-best-practices guidelines, making it well-related to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/improve-security

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Base automatically changed from feature/docusaurus-3.10.0 to develop May 5, 2026 13:23
@infeo
Copy link
Copy Markdown
Member Author

infeo commented May 5, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
package.json (1)

38-41: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Migrate overrides to canonical pnpm.overrides configuration.

The root-level "overrides" field is not recognized by pnpm; it is npm-specific syntax. pnpm only respects overrides declared within "pnpm": { "overrides": { … } }. These webpack plugin version pins are security-critical and may not be applied in the current configuration.

Migrate to:

-  "overrides": {
-    "copy-webpack-plugin": "14.0.0",
-    "css-minimizer-webpack-plugin": "8.0.0"
-  },
+  "pnpm": {
+    "overrides": {
+      "copy-webpack-plugin": "14.0.0",
+      "css-minimizer-webpack-plugin": "8.0.0"
+    }
+  },
🤖 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 `@package.json` around lines 38 - 41, Move the top-level "overrides" object
into pnpm's canonical location by creating or updating the "pnpm" object to
include an "overrides" entry that pins "copy-webpack-plugin" and
"css-minimizer-webpack-plugin" to the desired versions; replace the root-level
"overrides" block with a "pnpm": { "overrides": { "copy-webpack-plugin":
"14.0.0", "css-minimizer-webpack-plugin": "8.0.0" } } so pnpm will apply these
security-critical pins (update any code that referenced the old root "overrides"
key to use the "pnpm.overrides" location).
🧹 Nitpick comments (2)
pnpm-workspace.yaml (1)

12-15: minimumReleaseAge is not enforced when using --frozen-lockfile.

minimumReleaseAge is not enforced when pnpm install --frozen-lockfile is used if the lockfile already contains a dependency version that violates the constraint — once a violating version is present in pnpm-lock.yaml, pnpm allows installation to proceed without validation.

Because CI uses pnpm install --frozen-lockfile, this setting only gates pnpm add / pnpm update on developer machines. The protection is still valuable (prevents adding too-fresh packages to the lockfile in the first place), but the team should know the CI install doesn't re-validate ages.

🤖 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 `@pnpm-workspace.yaml` around lines 12 - 15, Add a note explaining that
minimumReleaseAge in pnpm-workspace.yaml is not enforced when CI runs pnpm with
--frozen-lockfile, and introduce a CI validation job (e.g.,
validate-minimum-release-age) that reads minimumReleaseAge from
pnpm-workspace.yaml and parses pnpm-lock.yaml to ensure none of the resolved
package versions are newer than the configured cooldown; if any are too fresh,
fail the job. Update documentation/comments near the minimumReleaseAge entry and
add a small script (invoked by the CI job) that references minimumReleaseAge and
pnpm-lock.yaml to perform the age check so CI enforces the same policy even with
--frozen-lockfile.
package.json (1)

5-5: ⚡ Quick win

Add Corepack integrity hash to the packageManager field.

The current "pnpm@10.33.2" tells Corepack which version to activate, but without a hash Corepack relies on signature verification—a potential gap when keys rotate or aren't available. Corepack supports an optional hash suffix for deterministic, tamper-resistant installs.

Update the packageManager field to include the sha512 integrity hash:

Diff
-  "packageManager": "pnpm@10.33.2",
+  "packageManager": "pnpm@10.33.2+sha512.qQ+vb+6rca1sblf5Tg/hoS9dzCLNdU20CulZPraj4LaxLjVAIYuzeuCDQEsfLObbKkEh6XmCm0r/lLmfSdoc+A==",
🤖 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 `@package.json` at line 5, Update the packageManager field value to include the
Corepack integrity hash so installs are deterministic and verifiable: replace
the current "packageManager": "pnpm@10.33.2" value with the same version
suffixed by the sha512 integrity token (format: "pnpm@10.33.2
(sha512-<base64-hash>)"), ensuring you preserve the exact version string and
only append the integrity hash in parentheses.
🤖 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 `@README.md`:
- Line 16: Replace the Quick Start command in README.md that currently shows
"pnpm install --frozen-lockfile" with the plain "pnpm install" so it matches
CONTRIBUTING.md and won't abort when contributors add dependencies; ensure the
README uses the bare pnpm install command (remove the --frozen-lockfile flag)
and, if desired, add an optional note explaining when to use --frozen-lockfile
for CI or reproducible installs.
- Line 20: Update the README Quick Start line that currently says
"http://localhost:3000" to the correct port used by the start script; the start
script in package.json passes --port 8000, so change the URL to
"http://localhost:8000" (or make it dynamic to reference the start script/PORT
variable) to match the actual behavior of the start command.

In `@scripts/check-lockfile.sh`:
- Around line 36-41: The current yq regex used to build bad_specs (in the
bad_specs=$(yq '...' "$LOCK_FILE") call) includes the literal "workspace:"
protocol which will flag valid pnpm first-party references; update the yq
expression to stop treating workspace references as an external/banned protocol
(either remove "workspace:" from the alternation list or replace it with a more
specific negative lookahead such as workspace:(?!(\*|\^)) so only non-workspace
uses are matched), keeping the rest of the protocols intact so CI no longer
false-positive fails on legitimate workspace:* or workspace:^ importer
specifiers.

---

Outside diff comments:
In `@package.json`:
- Around line 38-41: Move the top-level "overrides" object into pnpm's canonical
location by creating or updating the "pnpm" object to include an "overrides"
entry that pins "copy-webpack-plugin" and "css-minimizer-webpack-plugin" to the
desired versions; replace the root-level "overrides" block with a "pnpm": {
"overrides": { "copy-webpack-plugin": "14.0.0", "css-minimizer-webpack-plugin":
"8.0.0" } } so pnpm will apply these security-critical pins (update any code
that referenced the old root "overrides" key to use the "pnpm.overrides"
location).

---

Nitpick comments:
In `@package.json`:
- Line 5: Update the packageManager field value to include the Corepack
integrity hash so installs are deterministic and verifiable: replace the current
"packageManager": "pnpm@10.33.2" value with the same version suffixed by the
sha512 integrity token (format: "pnpm@10.33.2 (sha512-<base64-hash>)"), ensuring
you preserve the exact version string and only append the integrity hash in
parentheses.

In `@pnpm-workspace.yaml`:
- Around line 12-15: Add a note explaining that minimumReleaseAge in
pnpm-workspace.yaml is not enforced when CI runs pnpm with --frozen-lockfile,
and introduce a CI validation job (e.g., validate-minimum-release-age) that
reads minimumReleaseAge from pnpm-workspace.yaml and parses pnpm-lock.yaml to
ensure none of the resolved package versions are newer than the configured
cooldown; if any are too fresh, fail the job. Update documentation/comments near
the minimumReleaseAge entry and add a small script (invoked by the CI job) that
references minimumReleaseAge and pnpm-lock.yaml to perform the age check so CI
enforces the same policy even with --frozen-lockfile.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 00e8f771-1eaa-49e6-a6be-0c6d695f87c7

📥 Commits

Reviewing files that changed from the base of the PR and between b643900 and 486d5c5.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • .github/CONTRIBUTING.md
  • .github/dependabot.yml
  • .github/workflows/deploy.yml
  • .gitignore
  • README.md
  • package.json
  • pnpm-workspace.yaml
  • scripts/check-lockfile.sh
  • scripts/check-pm.js

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread scripts/check-lockfile.sh Outdated
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.

1 participant