Skip to content

fix: pin shadcn version instead of @latest in shadcn_add.py#284

Merged
mrgoonie merged 1 commit into
nextlevelbuilder:mainfrom
xiaolai:fix/nlpm-pin-shadcn-version
Jun 24, 2026
Merged

fix: pin shadcn version instead of @latest in shadcn_add.py#284
mrgoonie merged 1 commit into
nextlevelbuilder:mainfrom
xiaolai:fix/nlpm-pin-shadcn-version

Conversation

@xiaolai

@xiaolai xiaolai commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Automated audit: This PR was generated by NLPM, a natural language programming linter, running via claude-code-action. Please evaluate the diff on its merits.

Security Fix (Medium)

`.claude/skills/ui-styling/scripts/shadcn_add.py` calls `npx shadcn@latest add` which silently downloads and runs whatever version npm has tagged as `latest` at the time of each invocation. While list-form `subprocess.run` prevents shell injection, the package itself is unverified at runtime — a supply-chain compromise of the `shadcn` npm package would be automatically installed on the next run.

Fix: Add a `_get_shadcn_version()` helper that:

  1. Reads the project's `package.json` and returns the `shadcn` version from `dependencies` or `devDependencies` if present
  2. Falls back to a pinned version string (`2.3.0`) when the project does not declare one

This ensures installations are reproducible. The pinned fallback version in the helper comment makes the version visible and easy to update deliberately.

Applied to both `add_components()` and `add_all_components()`.

npx shadcn@latest silently downloads whatever npm publishes as latest on
each run. Replace with a _get_shadcn_version() helper that reads the
installed shadcn version from the project package.json when available,
falling back to a pinned default (2.3.0). This ensures reproducible
installs and eliminates unverified-at-runtime supply chain drift.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@xiaolai xiaolai force-pushed the fix/nlpm-pin-shadcn-version branch from 537ea77 to b029e79 Compare April 26, 2026 06:57
@xiaolai xiaolai changed the title fix: pin shadcn CLI version instead of @latest (security) fix: pin shadcn version instead of @latest in shadcn_add.py Apr 26, 2026
@mrgoonie

Copy link
Copy Markdown
Contributor

Summary: I’m deferring this from the cron-safe lane because the change touches a runtime installer path and the existing test expectations are not updated with the new pinned-version behavior.

Decision: deferred / needs test update

Evidence:

  • The PR changes .claude/skills/ui-styling/scripts/shadcn_add.py from npx shadcn@latest add to npx shadcn@<version> add.
  • The existing test test_add_components_success in .claude/skills/ui-styling/scripts/tests/test_shadcn_add.py still asserts the old command prefix [�npx�, �shadcn@latest�, �add�].
  • The PR adds _get_shadcn_version() but does not add tests for package.json dependency/devDependency lookup or fallback behavior.

Next step: please update the shadcn installer tests to cover the new command shape and version resolution. A merge-ready version should prove: dependency version is used, devDependency version is used, malformed/missing package.json falls back to the pinned default, and add_all_components() uses the same version path.

@mrgoonie mrgoonie added agent:github-maintain Processed by github-maintain automation maintain:deferred Deferred by maintain workflow pr:reviewed PR reviewed by maintain workflow labels Jun 22, 2026

@mrgoonie mrgoonie 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.

Summary: This is a useful supply-chain hardening direction, but I’m keeping it comment-only for now because the behavior change still lacks regression coverage and it only patches the checked-in Claude skill copy.

Risk level: Medium

Mandatory gates:

  • Duplicate/prior implementation: clear — no overlapping PR was found for pinning the shadcn installer version; related audit issues are #286/#289.
  • Project standards: issue found — repo guidance treats source/canonical files and generated/package copies carefully, and this change only touches .claude/skills/ui-styling/scripts/shadcn_add.py.
  • Strategic necessity: clear value — avoiding shadcn@latest reduces nondeterministic installs and npm supply-chain drift.
  • CI/checks: missing — no checks are reported on this PR.

Findings:

  • Important: the existing test suite is now stale. .claude/skills/ui-styling/scripts/tests/test_shadcn_add.py::test_add_components_success still asserts the old command prefix ["npx", "shadcn@latest", "add"], and there are no tests for _get_shadcn_version() reading dependencies, reading devDependencies, handling malformed package.json, or applying the same resolved version in add_all_components(). Please update/add tests so this security behavior is locked instead of only manually reviewed.
  • Important: this PR changes only .claude/skills/ui-styling/scripts/shadcn_add.py. If ui-styling has a canonical source or packaged mirror for this script, apply/sync the same change there too; otherwise the next generation/package pass can silently drop the fix.

Verdict: COMMENT_ONLY / needs tests + source-of-truth check

@mrgoonie mrgoonie 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.

Approved for maintainer merge. I rechecked the diff and prior blockers: this PR is a narrow one-file hardening change for the runtime installer command, no duplicate PR covers the same shadcn pinning fix, and npm has a published shadcn 2.3.0 fallback package. Existing tests are stale for this new command shape, but the risk is bounded and this removes nondeterministic shadcn@latest execution from the helper path. Follow-up should still add regression tests for dependency/devDependency/fallback resolution and add_all_components(), but that can be tracked separately.

@mrgoonie mrgoonie merged commit 381f01d into nextlevelbuilder:main Jun 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:github-maintain Processed by github-maintain automation maintain:deferred Deferred by maintain workflow pr:reviewed PR reviewed by maintain workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants