From cc40a65c5d32b6c3d4821338287e95c5dd54104e Mon Sep 17 00:00:00 2001 From: Enrique Sanchez Cardoso Date: Thu, 29 Jan 2026 01:48:46 +0100 Subject: [PATCH] feat: detailed permission denied message on push (#10941) Resolves: - #10497 - #10496 - #10499 Update the message Forgejo is showing when an user is not allowed to push to a repo: ``` remote: Forgejo: User 'username' is not allowed to push to 'branchname' in 'repo'. remote: If you instead wanted to create a pull request to the branch 'branchname', please use: remote: git push origin HEAD:refs/for/branchname/choose-a-descriptor remote: You might want to replace 'origin' with the name of your Git remote if it is different from origin. You can freely choose the descriptor to set it to a topic. remote: You can learn about creating pull requests with AGit in the docs: https://forgejo.org/docs/latest/user/agit-support/ ``` ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. *The decision if the pull request will be shown in the release notes is up to the mergers / release team.* The content of the `release-notes/.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead. ## Release notes - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/10941): detailed permission denied message on push Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10941 Reviewed-by: Mathieu Fenniak Co-authored-by: Enrique Sanchez Cardoso Co-committed-by: Enrique Sanchez Cardoso --- routers/private/hook_pre_receive.go | 12 +++++++++++- tests/integration/git_push_test.go | 23 ++++++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 217e5d817f..c6c6ad5bb0 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "os" + "strings" "forgejo.org/models" asymkey_model "forgejo.org/models/asymkey" @@ -72,8 +73,17 @@ func (ctx *preReceiveContext) AssertCanWriteCode() bool { if ctx.Written() { return false } + var sb strings.Builder + fmt.Fprintf(&sb, "User '%s' is not allowed to push to branch '%s' in '%s/%s'.", ctx.user.Name, ctx.branchName, ctx.Repo.Repository.OwnerName, ctx.Repo.Repository.Name) + + if ctx.CanCreatePullRequest() { + fmt.Fprintf(&sb, "\nIf you instead wanted to create a pull request to the branch '%s', please use:", ctx.branchName) + fmt.Fprintf(&sb, "\ngit push origin HEAD:refs/for/%s/choose-a-descriptor", ctx.branchName) + sb.WriteString("\nYou might want to replace 'origin' with the name of your Git remote if it is different from origin. You can freely choose the descriptor to set it to a topic.") + sb.WriteString("\nYou can learn about creating pull requests with AGit in the docs: https://forgejo.org/docs/latest/user/agit-support/") + } ctx.JSON(http.StatusForbidden, private.Response{ - UserMsg: "User permission denied for writing.", + UserMsg: sb.String(), }) return false } diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index 61e2a65f97..bf8ad8ac12 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -261,14 +261,31 @@ func testOptionsGitPush(t *testing.T, u *url.URL) { require.False(t, repo.IsTemplate) }) - // create a collaborator with write access + // create a collaborator user collaborator := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) u.User = url.UserPassword(collaborator.LowerName, userPassword) doGitAddRemote(gitPath, "collaborator", u)(t) + + t.Run("User without write access is not allowed to push", func(t *testing.T) { + pushLogChecker, cleanup := test.NewLogChecker("ssh", log.ERROR) + pushLogChecker.Filter("User 'user5' is not allowed to push to branch 'branch3' in 'user2/repo-to-push'.") + pushLogChecker.Filter("If you instead wanted to create a pull request to the branch 'branch3', please use:") + pushLogChecker.Filter("git push origin HEAD:refs/for/branch3/choose-a-descriptor") + pushLogChecker.Filter("You might want to replace 'origin' with the name of your Git remote if it is different from origin. You can freely choose the descriptor to set it to a topic.") + pushLogChecker.Filter("You can learn about creating pull requests with AGit in the docs: https://forgejo.org/docs/latest/user/agit-support/") + defer cleanup() + branchName := "branch3" + doGitCreateBranch(gitPath, branchName)(t) + doGitPushTestRepositoryFail(gitPath, "collaborator", branchName)(t) + pushLogFiltered, _ := pushLogChecker.Check(5 * time.Second) + assert.True(t, pushLogFiltered[0]) + }) + + // give write access to the collaborator repo_module.AddCollaborator(db.DefaultContext, repo, collaborator) t.Run("Collaborator with write access is allowed to push", func(t *testing.T) { - branchName := "branch3" + branchName := "branch4" doGitCreateBranch(gitPath, branchName)(t) doGitPushTestRepository(gitPath, "collaborator", branchName)(t) }) @@ -280,7 +297,7 @@ func testOptionsGitPush(t *testing.T, u *url.URL) { sshLogChecker, cleanup := test.NewLogChecker("ssh", log.ERROR) sshLogChecker.Filter("permission denied for changing repo settings") defer cleanup() - branchName := "branch4" + branchName := "branch5" doGitCreateBranch(gitPath, branchName)(t) doGitPushTestRepositoryFail(gitPath, "collaborator", branchName, "-o", "repo.private=true", "-o", "repo.template=true")(t) repo, err = repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, user.Name, "repo-to-push")