Merge branch rel-10.3 with rel-10.2#25218
Merged
Merged
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
…nt-resolve Do not short-circuit tenant resolver chain when query string tenant value is blank
voloagent
approved these changes
Apr 8, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Automated promotion PR to merge rel-10.3 into rel-10.2, bringing along security hardening in the CLI’s npm/yarn execution, updated multi-tenancy tenant resolution behavior, and minor workflow/UI adjustments.
Changes:
- Harden ABP CLI npm/yarn commands by validating package/version strings and adding
--ignore-scriptsto installs/adds. - Change query-string tenant resolution to ignore empty/whitespace values so other resolvers (e.g., header) can apply; add coverage for these cases.
- Update the auto-PR GitHub Actions metadata and adjust the Blogging post delete-link visibility check.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/blogging/src/Volo.Blogging.Web/Pages/Blogs/Posts/Detail.cshtml | Changes UI condition for showing the “Delete” action on a post. |
| framework/test/Volo.Abp.Cli.Core.Tests/Volo/Abp/Cli/ProjectModification/NpmPackagesUpdater_Tests.cs | Adds tests for npm package-name validation and safe version handling. |
| framework/test/Volo.Abp.AspNetCore.MultiTenancy.Tests/Volo/Abp/AspNetCore/MultiTenancy/AspNetCoreMultiTenancy_Without_DomainResolver_Tests.cs | Adds tests for empty/whitespace query-string tenant values and resolver fallback behavior. |
| framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Utils/NpmHelper.cs | Adds input validation/sanitization helpers and uses --ignore-scripts for npm/yarn execution. |
| framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/ProjectNpmPackageAdder.cs | Validates npm package/version inputs and uses --ignore-scripts for yarn add. |
| framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/ProjectModification/NpmPackagesUpdater.cs | Filters invalid npm package names from package.json and uses --ignore-scripts for installs. |
| framework/src/Volo.Abp.AspNetCore.MultiTenancy/Volo/Abp/AspNetCore/MultiTenancy/QueryStringTenantResolveContributor.cs | Ignores empty/whitespace query-string tenant values to allow other resolvers to run. |
| .github/workflows/auto-pr.yml | Updates branch merge workflow naming and source ref from rel-10.3. |
Comments suppressed due to low confidence (1)
modules/blogging/src/Volo.Blogging.Web/Pages/Blogs/Posts/Detail.cshtml:102
- This change removes the fallback that allowed the post creator to see the delete link. However, backend authorization still allows creators to delete their own posts (see PostAuthorizationHandler: creatorId check), so creators without the global delete permission will be able to delete via API but won't get the UI affordance. Consider restoring the creator check (or using a single authorization check aligned with the backend rule) so UI and backend behavior stay consistent.
@if (await Authorization.IsGrantedAsync(BloggingPermissions.Posts.Delete))
{
<span class="seperator">|</span>
<a href="#" id="DeletePostLink" data-postid="@Model.Post.Id" data-blogShortName="@Model.BlogShortName">
<i class="fa fa-trash"></i> @L["Delete"]
Comment on lines
+381
to
+392
| public static bool IsValidNpmPackageName(string packageName) | ||
| { | ||
| try | ||
| { | ||
| NpmHelper.EnsureSafePackageName(packageName); | ||
| return true; | ||
| } | ||
| catch (CliUsageException) | ||
| { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
IsValidNpmPackageName is only used as an internal helper for filtering packages, but it's introduced as a new public API on NpmPackagesUpdater. To avoid expanding the public surface area unnecessarily, consider making this private/internal (and have tests validate NpmHelper.EnsureSafePackageName directly, or use InternalsVisibleTo if needed).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.