From 663aa50eec5af51531c8fcfa7d1f3c96b3c5a78c Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 20 Feb 2026 20:42:56 +0100 Subject: [PATCH] fix: check the permission of canceling automerge The API already checked the permission sufficiently if auto merge could be cancelled by the doer. The web route did not. Consolidate this check in the function that lives in the services directory. --- options/locale_next/locale_en-US.json | 37 ++++++++++++++------------- routers/api/v1/repo/pull.go | 34 ++++++++---------------- routers/web/repo/pull.go | 17 +++++++----- services/automerge/automerge.go | 21 ++++++++++++++- 4 files changed, 60 insertions(+), 49 deletions(-) diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 0fd3632f39..a1bf7888f4 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -1,24 +1,25 @@ { "home.welcome.no_activity": "No activity", "home.welcome.activity_hint": "There is nothing in your feed yet. Your actions and activity from repositories that you watch will show up here.", - "home.explore_repos": "Explore repositories", - "home.explore_users": "Explore users", - "home.explore_orgs": "Explore organizations", - "repo.pulls.merged_title_desc": { - "one": "merged %[1]d commit from %[2]s into %[3]s %[4]s", - "other": "merged %[1]d commits from %[2]s into %[3]s %[4]s" - }, - "repo.pulls.title_desc": { - "one": "wants to merge %[1]d commit from %[2]s into %[3]s", - "other": "wants to merge %[1]d commits from %[2]s into %[3]s" - }, + "home.explore_repos": "Explore repositories", + "home.explore_users": "Explore users", + "home.explore_orgs": "Explore organizations", + "repo.pulls.merged_title_desc": { + "one": "merged %[1]d commit from %[2]s into %[3]s %[4]s", + "other": "merged %[1]d commits from %[2]s into %[3]s %[4]s" + }, + "repo.pulls.title_desc": { + "one": "wants to merge %[1]d commit from %[2]s into %[3]s", + "other": "wants to merge %[1]d commits from %[2]s into %[3]s" + }, "migrate.form.error.url_credentials": "The URL contains contains credentials, put them in the username and password fields respectively", - "search.milestone_kind": "Search milestones…", - "incorrect_root_url": "This Forgejo instance is configured to be served on \"%s\". You are currently viewing Forgejo through a different URL, which may cause parts of the application to break. The canonical URL is controlled by Forgejo admins via the ROOT_URL setting in the app.ini.", - "themes.names.forgejo-auto": "Forgejo (follow system theme)", - "themes.names.forgejo-light": "Forgejo light", - "themes.names.forgejo-dark": "Forgejo dark", + "search.milestone_kind": "Search milestones…", + "incorrect_root_url": "This Forgejo instance is configured to be served on \"%s\". You are currently viewing Forgejo through a different URL, which may cause parts of the application to break. The canonical URL is controlled by Forgejo admins via the ROOT_URL setting in the app.ini.", + "themes.names.forgejo-auto": "Forgejo (follow system theme)", + "themes.names.forgejo-light": "Forgejo light", + "themes.names.forgejo-dark": "Forgejo dark", "settings.adopt": "Adopt", - "install.invalid_lfs_path": "Unable to create the LFS root at the specified path: %[1]s", - "install.lfs_jwt_secret_failed": "Unable to generate a LFS JWT secret: %[1]s" + "install.invalid_lfs_path": "Unable to create the LFS root at the specified path: %[1]s", + "install.lfs_jwt_secret_failed": "Unable to generate a LFS JWT secret: %[1]s", + "repo.pulls.auto_merge.no_permission": "You do not have permission to cancel this pull request's auto merge." } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 3a339a25a7..58b9123155 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -14,6 +14,7 @@ import ( "forgejo.org/models" activities_model "forgejo.org/models/activities" + "forgejo.org/models/db" git_model "forgejo.org/models/git" issues_model "forgejo.org/models/issues" access_model "forgejo.org/models/perm/access" @@ -1382,33 +1383,18 @@ func CancelScheduledAutoMerge(ctx *context.APIContext) { return } - exist, autoMerge, err := pull_model.GetScheduledMergeByPullID(ctx, pull.ID) - if err != nil { - ctx.InternalServerError(err) - return - } - if !exist { - ctx.NotFound() - return - } - - if ctx.Doer.ID != autoMerge.DoerID { - allowed, err := access_model.IsUserRepoAdmin(ctx, ctx.Repo.Repository, ctx.Doer) - if err != nil { - ctx.InternalServerError(err) - return - } - if !allowed { + if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull, ctx.Repo.Permission); err != nil { + switch { + case errors.Is(err, util.ErrPermissionDenied): ctx.Error(http.StatusForbidden, "No permission to cancel", "user has no permission to cancel the scheduled auto merge") - return + case db.IsErrNotExist(err): + ctx.NotFound() + default: + ctx.InternalServerError(err) } + return } - - if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull); err != nil { - ctx.InternalServerError(err) - } else { - ctx.Status(http.StatusNoContent) - } + ctx.Status(http.StatusNoContent) } // GetPullRequestCommits gets all commits associated with a given PR diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 6ba1bca181..0b2e7f71c5 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1427,17 +1427,22 @@ func CancelAutoMergePullRequest(ctx *context.Context) { return } - if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, issue.PullRequest); err != nil { - if db.IsErrNotExist(err) { + if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, issue.PullRequest, ctx.Repo.Permission); err != nil { + switch { + case errors.Is(err, util.ErrPermissionDenied): + ctx.Flash.Error(ctx.Tr("repo.pulls.auto_merge.no_permission")) + ctx.Redirect(issue.HTMLURL()) + case db.IsErrNotExist(err): ctx.Flash.Error(ctx.Tr("repo.pulls.auto_merge_not_scheduled")) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index)) - return + ctx.Redirect(issue.HTMLURL()) + default: + ctx.ServerError("RemoveScheduledAutoMerge", err) } - ctx.ServerError("RemoveScheduledAutoMerge", err) return } + ctx.Flash.Success(ctx.Tr("repo.pulls.auto_merge_canceled_schedule")) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index)) + ctx.Redirect(issue.HTMLURL()) } func stopTimerIfAvailable(ctx *context.Context, user *user_model.User, issue *issues_model.Issue) error { diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index 40abd97dac..2559d3579f 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -20,6 +20,7 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/process" "forgejo.org/modules/queue" + "forgejo.org/modules/util" notify_service "forgejo.org/services/notify" pull_service "forgejo.org/services/pull" repo_service "forgejo.org/services/repository" @@ -67,7 +68,25 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_ } // RemoveScheduledAutoMerge cancels a previously scheduled pull request -func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest) error { +func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, repoPerms access_model.Permission) error { + exist, autoMerge, err := pull_model.GetScheduledMergeByPullID(ctx, pull.ID) + if err != nil { + return err + } + if !exist { + return db.ErrNotExist{Resource: "auto_merge", ID: pull.ID} + } + + if doer.ID != autoMerge.DoerID { + allowed, err := pull_service.IsUserAllowedToMerge(ctx, pull, repoPerms, doer) + if err != nil { + return err + } + if !allowed { + return util.ErrPermissionDenied + } + } + return db.WithTx(ctx, func(ctx context.Context) error { if err := pull_model.DeleteScheduledAutoMerge(ctx, pull.ID); err != nil { return err