Skip to content

Harden the organization check install - #5

Merged
silveltman merged 1 commit into
mainfrom
security/pnpm-install-hardening
Oct 2, 2026
Merged

silveltman merged 1 commit into
mainfrom
security/pnpm-install-hardening

Conversation

@silveltman

@silveltman silveltman commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Hardens the install in the organization Check workflow that the Merge policy ruleset requires on every pull request.

Rebased on main after #3 (checkout and setup-node 7) merged. Replaces #4, which GitHub closed when #3's branch was deleted.

What changes

  • Install always runs. Before, a cache hit restored node_modules and skipped pnpm install, so a run never checked the lockfile against the repository's pnpm settings or the registry. Now only the pnpm store (downloaded packages) is cached, keyed on pnpm-lock.yaml, and pnpm install --frozen-lockfile runs every time. That is where trustPolicy, the strict release age and ignorePnpmfile from the per-repository PRs are enforced.
  • --ignore-pnpmfile added to the install, and pnpm_config_ignore_scripts / pnpm_config_ignore_pnpmfile set for the job, so installs started from a check script get the same protection. Explicit pnpm run scripts still run.
  • Actions pinned to commit SHAs with the version in a comment: actions/checkout v7.0.1 and actions/setup-node v7.0.0 (the current v7 tags, as Use checkout and setup-node 7 in the check workflow #3 set), actions/cache v6.1.0 (the current v6 tag).
  • Permissions stay as they were: contents: read for the check, write only on the CloudCannon auto-merge job, which is unchanged.

Testing

  • actionlint 1.7.12 and Prettier pass.
  • pnpm store path --silent prints only the store path on pnpm 11.28.0 and 12.8.1.
  • pnpm install --frozen-lockfile --ignore-scripts --ignore-pnpmfile with pnpm 12.8.1, the version all 43 repositories now pin, passes on each repository's security PR rebased on current main.
  • Not tested end to end: the ruleset runs this workflow from this repository's main, so other repositories only use it after merge. This PR's own Check finds no package and exits with the "nothing to run" notice, so it does not exercise the new steps. Watch the first PRs after merging.

Risks

  • Merge order with the repository PRs does not matter: without a repository's security PR the install uses pnpm defaults, and with it the frozen install was tested to pass.
  • A run without a warm store downloads all packages; later runs restore the store. Expect slightly slower checks than a node_modules cache hit, in exchange for a verified install.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated the automated validation workflow to install dependencies consistently and reuse its package cache.
    • Pinned workflow setup actions to specific versions and tightened installation settings. These changes affect the project’s checks; no user-facing product changes are included.

Always run the frozen install, cache the pnpm store instead of node_modules,
skip pnpmfiles, protect nested installs, and pin actions to commit SHAs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@silveltman
silveltman marked this pull request as ready for review October 2, 2026 18:57
@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 828947e0-46be-49ec-b01e-afb6574d7b15

📥 Commits

Reviewing files that changed from the base of the PR and between 6b487d5 and 1a3d46b.

📒 Files selected for processing (1)
  • .github/workflows/check.yml

Limit details: You’ve used all 10 included reviews currently available.


📝 Walkthrough

Walkthrough

The check workflow pins the checkout and setup-node actions to commits. It sets pnpm options to ignore scripts and pnpmfiles. It replaces the node_modules cache with a pnpm store cache keyed by runner OS and lockfile hash. When a check script is found, the workflow always installs dependencies with a frozen lockfile and ignores scripts and pnpmfiles.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 1a3d4

No actionable merge-blocking issue is established. The workflow’s behavior in consuming repositories should be observed after rollout.

Architecture Summary

Architecture risk: 🔵 Low · up to 1a3d4

The changed surface does not map to a changed system, dependency edge, entrypoint, or external dependency.

Changed systems: None identified.

Architecture concerns
No architecture-level concerns identified.

Review details

Before / after behavior

  • observed — Modified behavior in .github/workflows/check.yml: The job adds pnpm environment settings to ignore scripts and pnpmfiles, and pins the checkout action to a commit instead of using the v7 tag.
  • observed — Modified behavior in .github/workflows/check.yml: The setup-node action is pinned to a commit instead of using the v7 tag.
  • observed — Modified behavior in .github/workflows/check.yml: The workflow removes the node_modules cache keyed by lockfile and package manifests, replacing it with a step that obtains the pnpm store path.
  • observed — Modified behavior in .github/workflows/check.yml: A pnpm-store cache is added, keyed by runner OS and lockfile hash, with an OS-specific restore prefix.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: hardening the organization check workflow's install process.
Description check ✅ Passed The description directly explains the workflow changes, testing, risks, and known end-to-end testing limitation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Usage-based review receipt

  • Mode: Continue automatically
  • Reviewed files: 1
  • Waived: $0.25 (charged $0.00)
  • View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.


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

@silveltman
silveltman merged commit 5d22144 into main Oct 2, 2026
3 checks passed
@silveltman
silveltman deleted the security/pnpm-install-hardening branch October 2, 2026 19:03
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