500 error if PR merge is rejected by pre-receive hook (exit 1) #4875

New Issue

GiteaMirror · 2025-11-02T06:05:43-06:00

GiteaMirror commented

2025-11-02 06:05:43 -06:00

Originally created by @mshgh on GitHub (Feb 17, 2020).

Gitea version (or commit ref): 1.11.1 (docker image)
Git version: not relevant
Operating system: not relevant
Database (use [x]):
- PostgreSQL
- MySQL
- MSSQL
- SQLite
Can you reproduce the bug at https://try.gitea.io:
- Yes (provide example URL)
- No
- Not relevant
Log gist:

Description

I tried to test pre-receive git hook (by editing via gitea web interface) and setup simple rejection

#!/bin/sh
echo "Forced rejection" >&2
exit 1

every direct (again via web interface) file modification is rejected as expected with the error message above. However if I try to merge a pull request I get 500 error page instead.

Originally created by @mshgh on GitHub (Feb 17, 2020). - Gitea version (or commit ref): 1.11.1 (docker image) - Git version: not relevant - Operating system: not relevant - Database (use `[x]`): - [ ] PostgreSQL - [x] MySQL - [ ] MSSQL - [ ] SQLite - Can you reproduce the bug at https://try.gitea.io: - [ ] Yes (provide example URL) - [ ] No - [x] Not relevant - Log gist: ## Description I tried to test *pre-receive* git hook (by editing via gitea web interface) and setup simple rejection ```javascript #!/bin/sh echo "Forced rejection" >&2 exit 1 ``` every direct (again via web interface) file modification is rejected as expected with the error message above. However if I try to merge a pull request I get 500 error page instead.

GiteaMirror added the type/bug label 2025-11-02 06:05:43 -06:00

GiteaMirror closed this issue

2025-11-02 06:05:43 -06:00

GiteaMirror commented

2025-11-02 06:05:44 -06:00

@zeripath commented on GitHub (Feb 17, 2020):

Yes that's because pre-receive hooks run on merge, crud and repository creation too

The simplest way you can detect internal actions is by looking at the contents of SSH_ORIGINAL_COMMAND. We have however not completely specd out the what contents of this should be and would like to think a bit more carefully about how this should work

@zeripath commented on GitHub (Feb 17, 2020): Yes that's because pre-receive hooks run on merge, crud and repository creation too The simplest way you can detect internal actions is by looking at the contents of SSH_ORIGINAL_COMMAND. We have however not completely specd out the what contents of this should be and would like to think a bit more carefully about how this should work

GiteaMirror commented

2025-11-02 06:05:44 -06:00

@mshgh commented on GitHub (Feb 18, 2020):

I seem to be not clear enough. I actually want to abort internal gitea operation (under certain conditions). See below what happens if I try to edit a file. I expect the same error message for the Pull Request page, but 500 error page is shown instead. I believe this is a bug.

@mshgh commented on GitHub (Feb 18, 2020): I seem to be not clear enough. I actually want to abort internal gitea operation (under certain conditions). See below what happens if I try to edit a file. I expect the same error message for the Pull Request page, but 500 error page is shown instead. I believe this is a bug. ![EditReadmeRejected2](https://user-images.githubusercontent.com/36337240/74744296-466c5980-5262-11ea-8a7e-43df34a28149.png)

GiteaMirror commented

2025-11-02 06:05:44 -06:00

@mshgh commented on GitHub (Feb 19, 2020):

@zeripath @lunny this was labeled as a 'question', but please can you give this second thought? My hook is doing exactly what I intend to do - to prevent pull request merge if appropriate. However the user experience is disaster - 500 error page instead of a nice error message on the top of the pull request page where there is clean explanation (coming from my hook) why the merge was prevented.

From my personal point of view I see this as a bug as I have high expectations from gitea :-) But if you say "Enhancement" I would not complain. It just makes a lot of sense to me gitea behaves this way.

@mshgh commented on GitHub (Feb 19, 2020): @zeripath @lunny this was labeled as a 'question', but please can you give this second thought? My hook is doing exactly what I intend to do - to prevent pull request merge if appropriate. However the user experience is disaster - 500 error page instead of a nice error message on the top of the pull request page where there is clean explanation (coming from my hook) why the merge was prevented. From my personal point of view I see this as a bug as I have high expectations from gitea :-) But if you say "Enhancement" I would not complain. It just makes a lot of sense to me gitea behaves this way.

GiteaMirror commented

2025-11-02 06:05:45 -06:00

@lunny commented on GitHub (Feb 19, 2020):

500 should be a bug, we should prompt an error message.

@lunny commented on GitHub (Feb 19, 2020): 500 should be a bug, we should prompt an error message.

GiteaMirror commented

2025-11-02 06:05:45 -06:00

@zeripath commented on GitHub (Feb 19, 2020):

OK I think we should try to provide the rejection message but the message displayed above is bad - we should not display the real repository path.

@zeripath commented on GitHub (Feb 19, 2020): OK I think we should try to provide the rejection message but the message displayed above is bad - we should not display the real repository path.

GiteaMirror commented

2025-11-02 06:05:45 -06:00

@zeripath commented on GitHub (Feb 19, 2020):

Now the error messages in CRUD actions also clearly need to be sanitised. We should not be exposing the system filepath of the repositories.

@zeripath commented on GitHub (Feb 19, 2020): Now the error messages in CRUD actions also clearly need to be sanitised. We should not be exposing the system filepath of the repositories.

GiteaMirror commented

2025-11-02 06:05:45 -06:00

@guillep2k commented on GitHub (Feb 20, 2020):

Now the error messages in CRUD actions also clearly need to be sanitised. We should not be exposing the system filepath of the repositories.

I think we could shamelessly replace setting.RepoRootPath with "{repositories}" in the error output (a similar thing with the temporary path too). There are edge cases were the error message could be garbled, but it would probably be better than it currently is.

@guillep2k commented on GitHub (Feb 20, 2020): > > > Now the error messages in CRUD actions also clearly need to be sanitised. We should not be exposing the system filepath of the repositories. I think we could shamelessly replace `setting.RepoRootPath` with `"{repositories}"` in the error output (a similar thing with the temporary path too). There are edge cases were the error message could be garbled, but it would probably be better than it currently is.

GiteaMirror commented

2025-11-02 06:05:45 -06:00

@zeripath commented on GitHub (Feb 20, 2020):

I've just done that and extended my changes to affect web CRUD too.

@zeripath commented on GitHub (Feb 20, 2020): I've just done that and extended my changes to affect web CRUD too.

GiteaMirror commented

2025-11-02 06:05:46 -06:00

@mshgh commented on GitHub (Mar 19, 2020):

There is very curious bug in Gitea 1.11.3 docker image I can see. If the message provided from pre-receive hook ends with new line the error message shows the internal repository part again. See details how to reproduce below.

One more thing. The message cannot contain HTML anymore. As the whole message is center-aligned multi-line messages are not pretty. In my case I am showing list of commits preventing the merge. You can imagine how funny the list of commits with comments looks like if this is center-aligned. This is definitely minor, but when the bug above is being fixed can you consider to
a) allow HTML messages. GitHooks can be created by site admin only so I recon we can consider them safe
b) place the message into pre-fromatted block which itself is centered, but its content is left-aligned

OK case

#!/bin/sh

echo -e "Foo" >&2
exit 1

Merge Failed: The push was rejected with the following message:
Foo
Review the githooks for this repository

BAD case

#!/bin/sh

echo -e "Foo\n" >&2
exit 1

Merge Failed: The push was rejected with the following message:
Foo

To /data/git/repositories/gadmin/test.git
! [remote rejected] base -> master (pre-receive hook declined)
error: failed to push some refs to '/data/git/repositories/gadmin/test.git'
Review the githooks for this repository

@mshgh commented on GitHub (Mar 19, 2020): There is very curious bug in Gitea 1.11.3 docker image I can see. If the message provided from `pre-receive` hook ends with new line the error message shows the internal repository part again. See details how to reproduce below. One more thing. The message cannot contain HTML anymore. As the whole message is center-aligned multi-line messages are not pretty. In my case I am showing list of commits preventing the merge. You can imagine how funny the list of commits with comments looks like if this is center-aligned. This is definitely minor, but when the bug above is being fixed can you consider to a) allow HTML messages. GitHooks can be created by site admin only so I recon we can consider them safe b) place the message into pre-fromatted block which itself is centered, but its content is left-aligned OK case ``` bash #!/bin/sh echo -e "Foo" >&2 exit 1 ``` ``` Merge Failed: The push was rejected with the following message: Foo Review the githooks for this repository ``` BAD case ``` bash #!/bin/sh echo -e "Foo\n" >&2 exit 1 ``` ``` Merge Failed: The push was rejected with the following message: Foo To /data/git/repositories/gadmin/test.git ! [remote rejected] base -> master (pre-receive hook declined) error: failed to push some refs to '/data/git/repositories/gadmin/test.git' Review the githooks for this repository ```

GiteaMirror commented

2025-11-02 06:05:46 -06:00

@zeripath commented on GitHub (Mar 19, 2020):

@mshgh please try not to message on old bugs - it's very hard to keep up with the amount of notifications on github for gitea so it may get ignored.

I'll take a look though.

@zeripath commented on GitHub (Mar 19, 2020): @mshgh please try not to message on old bugs - it's very hard to keep up with the amount of notifications on github for gitea so it may get ignored. I'll take a look though.

GiteaMirror commented

2025-11-02 06:05:46 -06:00

@zeripath commented on GitHub (Mar 19, 2020):

Lets discuss message formatting and html in a new issue.

@zeripath commented on GitHub (Mar 19, 2020): Lets discuss message formatting and html in a new issue.

GiteaMirror commented

2025-11-02 06:05:46 -06:00

@mshgh commented on GitHub (Mar 20, 2020):

@zeripath thank you for noticing. I was considering this approach or new issue. Now I know better ;)

Separate issue created https://github.com/go-gitea/gitea/issues/10778

@mshgh commented on GitHub (Mar 20, 2020): @zeripath thank you for noticing. I was considering this approach or new issue. Now I know better ;) Separate issue created https://github.com/go-gitea/gitea/issues/10778

GiteaMirror referenced this issue

2025-11-02 13:53:32 -06:00

[PR #4875] [MERGED] Remove feature `global code search` from GitLab CE #17530

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#4875