Skip to content

Sanitize package.json and prevent command injection in ABP CLI#25210

Merged
ebicoglu merged 5 commits intorel-10.2from
cli/sanitize-package-json-scripts
Apr 6, 2026
Merged

Sanitize package.json and prevent command injection in ABP CLI#25210
ebicoglu merged 5 commits intorel-10.2from
cli/sanitize-package-json-scripts

Conversation

@maliming
Copy link
Copy Markdown
Member

@maliming maliming commented Apr 5, 2026

  • Add --ignore-scripts flag to all npm/yarn install and add commands to prevent execution of potentially malicious lifecycle scripts from package.json.
  • Validate npm package names against a strict regex (@scope/package-name pattern) before passing them to shell commands, preventing OS command injection via crafted dependency names (e.g. @abp/core && calc.exe).
  • Add unit tests for package name validation covering both valid names and various injection patterns.

Fixes #25209

Copilot AI review requested due to automatic review settings April 5, 2026 09:38
@maliming maliming added this to the 10.2-patch milestone Apr 5, 2026
@maliming maliming marked this pull request as draft April 5, 2026 09:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens ABP CLI’s Node package operations by adding --ignore-scripts to npm/yarn install/add commands, reducing the risk of executing malicious lifecycle scripts from package.json (fixes #25209).

Changes:

  • Add --ignore-scripts to npm install and yarn execution in NpmHelper.
  • Add --ignore-scripts to npx yarn add calls used when modifying projects.
  • Add --ignore-scripts to install flows in NpmPackagesUpdater.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs Adds --ignore-scripts to npm/yarn helper commands.
framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/ProjectNpmPackageAdder.cs Adds --ignore-scripts to yarn add when adding packages to projects.
framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/NpmPackagesUpdater.cs Adds --ignore-scripts to yarn/npm installs during update workflows.

Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
@maliming maliming changed the title Add --ignore-scripts flag to npm/yarn commands in ABP CLI Sanitize package.json and prevent command injection in ABP CLI Apr 5, 2026
@maliming maliming requested a review from Copilot April 5, 2026 09:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Comment thread framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/ProjectNpmPackageAdder.cs:82

  • versionPostfix uses version != null, so an empty/whitespace version (which currently passes EnsureSafeVersion) becomes "@"/"@ ", producing an invalid yarn add command. Normalize version (trim + null if empty) or build the postfix with !string.IsNullOrWhiteSpace(version) so optional/empty versions behave correctly.
        if (!File.ReadAllText(packageJsonFilePath).Contains($"\"{npmPackage.Name}\""))
        {
            var versionPostfix = version != null ? $"@{version}" : string.Empty;

@maliming maliming marked this pull request as ready for review April 5, 2026 10:14
@maliming maliming requested a review from ebicoglu April 5, 2026 23:09
@ebicoglu ebicoglu merged commit f616cee into rel-10.2 Apr 6, 2026
4 of 5 checks passed
@ebicoglu ebicoglu deleted the cli/sanitize-package-json-scripts branch April 6, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants