Conversation
Update workflow to merge rel-10.3 with rel-10.2
Remove the ownership-based fallback that allowed post creators to delete their own posts in Detail.cshtml. Deletion now strictly requires BloggingPermissions.Posts.Delete, centralizing authorization on explicit permissions to enforce consistent access control.
Require delete permission for blog posts
…l, and add version tests
…-scripts Sanitize package.json and prevent command injection in ABP CLI
There was a problem hiding this comment.
Pull request overview
Automated merge PR from rel-10.3 into rel-10.2, bringing in workflow updates plus CLI hardening changes around npm/yarn execution and a small Blogging UI authorization tweak.
Changes:
- Tightens ABP CLI npm/yarn execution by adding input validation (package/version) and appending
--ignore-scriptsto install/add commands. - Filters out unsafe ABP npm package names during package.json scanning (with warning logs when skipped).
- Updates the auto-PR GitHub Actions workflow to merge
rel-10.3intorel-10.2and aligns Blogging post delete link visibility with permission checks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/blogging/src/Volo.Blogging.Web/Pages/Blogs/Posts/Detail.cshtml | Adjusts delete link visibility to rely on delete permission only. |
| framework/test/Volo.Abp.Cli.Core.Tests/Volo/Abp/Cli/ProjectModification/NpmPackagesUpdater_Tests.cs | Adds test coverage for npm package name safety and version safety validation. |
| framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs | Adds safe package/version validation utilities and applies --ignore-scripts to npm/yarn invocations. |
| framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/ProjectNpmPackageAdder.cs | Validates npm package/version inputs and applies --ignore-scripts when adding packages. |
| framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/NpmPackagesUpdater.cs | Filters unsafe package names from ABP-scoped dependencies and applies --ignore-scripts during installs. |
| .github/workflows/auto-pr.yml | Renames/repurposes the workflow to open PRs merging rel-10.3 into rel-10.2. |
| NpmHelper.EnsureSafePackageName(npmPackage.Name); | ||
| NpmHelper.EnsureSafeVersion(version); | ||
|
|
There was a problem hiding this comment.
EnsureSafeVersion(version) treats empty/whitespace as valid, but later versionPostfix is built with version != null, so an empty string will produce an invalid @ suffix (e.g., yarn add pkg@). Consider normalizing version to null/whitespace or using string.IsNullOrWhiteSpace(version) when constructing versionPostfix to avoid emitting @ with no version.
| NpmHelper.EnsureSafeVersion(version); | ||
|
|
||
| var versionPostfix = version != null ? $"@{version}" : string.Empty; | ||
|
|
||
| using (DirectoryHelper.ChangeCurrentDirectory(directory)) | ||
| { | ||
| Logger.LogInformation("yarn add " + npmPackage.Name + versionPostfix); | ||
| CmdHelper.RunCmd("npx yarn add " + npmPackage.Name + versionPostfix); | ||
| CmdHelper.RunCmd("npx yarn add " + npmPackage.Name + versionPostfix + " --ignore-scripts"); |
There was a problem hiding this comment.
versionPostfix is computed using version != null, so version = "" results in @ being appended (invalid npm spec). Since EnsureSafeVersion also allows empty/whitespace, consider using string.IsNullOrWhiteSpace(version) when building the postfix (or normalizing empty to null) to prevent generating package@.
| { | ||
| Logger.LogInformation($"Running Yarn on {directory}"); | ||
| CmdHelper.RunCmd($"npx yarn", directory); | ||
| CmdHelper.RunCmd($"npx yarn --ignore-scripts", directory); |
There was a problem hiding this comment.
Adding --ignore-scripts to the default yarn invocation disables lifecycle scripts (including the root project's postinstall). This can break repositories that rely on postinstall for setup (e.g., modules/cms-kit/angular/package.json runs npm run compile:ivy in postinstall). Consider making --ignore-scripts opt-in/configurable, or running the required project scripts explicitly after install when this flag is used.
| CmdHelper.RunCmd($"npx yarn --ignore-scripts", directory); | |
| CmdHelper.RunCmd("npx yarn", directory); |
| public static bool IsValidNpmPackageName(string packageName) | ||
| { | ||
| try | ||
| { | ||
| NpmHelper.EnsureSafePackageName(packageName); |
There was a problem hiding this comment.
IsValidNpmPackageName currently checks whether the name is safe for shell usage (via EnsureSafePackageName), not whether it’s a valid npm package name per npm’s naming rules. To avoid confusion for future callers, consider renaming to something like IsSafeNpmPackageName (or expanding validation to match npm’s actual spec).
This PR generated automatically to merge rel-10.3 with rel-10.2. Please review the changed files before merging to prevent any errors that may occur.