Fix verifyCommits error when push a new branch #26664

CaiCandong · 2023-08-22T13:34:40Z

Description

If a new branch is pushed, and the repository has a rule that would require signed commits for the new branch, the commit is rejected with a 500 error regardless of whether it's signed.

When pushing a new branch, the "old" commit is the empty ID (0000000000000000000000000000000000000000). verifyCommits has no provision for this and passes an invalid commit range to git rev-list. Prior to 1.19 this wasn't an issue because only pre-existing individual branches could be protected.

I was able to reproduce with try.gitea.io/CraigTest/test, which is set up with a blanket rule to require commits on all branches.

Fix #25565
Very thanks to @Craig-Holmquist-NTI for reporting the bug and suggesting an valid solution!

lunny · 2023-08-22T13:41:18Z

Could you add some test?

CaiCandong · 2023-08-22T13:49:56Z

Could you add some test?

how to add some test for it ?

routers/private/hook_verification.go

lunny · 2023-08-22T14:47:00Z

Could you add some test?

how to add some test for it ?

You can create a unit test for the function verifyCommits.

CaiCandong · 2023-08-23T08:38:13Z

I don't know how to prepare the test environment in the unit test, anyone help me to write the test?

wxiaoguang · 2023-08-23T15:19:32Z

Merge history is broken

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

lunny · 2023-08-24T01:02:32Z

Merge history is broken

Fixed

CaiCandong · 2023-08-24T10:56:20Z

This unit test requires the appropriate user data, GPG signature commit, and GPG Key to complete, which will take a bit of time to complete. Maybe we can merge this PR first (I've tested it locally with no problems) and I'll open a PR to supplement this test.@lunny
Done 53cc3d5

lunny · 2023-08-28T01:04:41Z

A small nit, some git files could be removed like routers/private/tests/repos/repo1_hook_verification/description and routers/private/tests/repos/repo1_hook_verification/exclude. Otherwise LGTM

CaiCandong · 2023-08-28T01:47:57Z

A small nit, some git files could be removed like routers/private/tests/repos/repo1_hook_verification/description and routers/private/tests/repos/repo1_hook_verification/exclude. Otherwise LGTM

Done dc24b94 (#26664)

@Craig-Holmquist-NTI

> ### Description > If a new branch is pushed, and the repository has a rule that would require signed commits for the new branch, the commit is rejected with a 500 error regardless of whether it's signed. > > When pushing a new branch, the "old" commit is the empty ID (0000000000000000000000000000000000000000). verifyCommits has no provision for this and passes an invalid commit range to git rev-list. Prior to 1.19 this wasn't an issue because only pre-existing individual branches could be protected. > > I was able to reproduce with [try.gitea.io/CraigTest/test](https://try.gitea.io/CraigTest/test), which is set up with a blanket rule to require commits on all branches. Fix go-gitea#25565 Very thanks to @Craig-Holmquist-NTI for reporting the bug and suggesting an valid solution! --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

* giteaofficial/main: Use `Set[Type]` instead of `map[Type]bool/struct{}`. (go-gitea#26804) Fix verifyCommits error when push a new branch (go-gitea#26664) Fix Uint8Array comparisons and update vitest (go-gitea#26805) Add various missing files-changed dependencies (go-gitea#26799) Improve flex list item padding (go-gitea#26779) Include the GITHUB_TOKEN/GITEA_TOKEN secret for fork pull requests (go-gitea#26759) feat(API): add route and implementation for creating/updating repository secret (go-gitea#26766) Replace deprecated `elliptic.Marshal` (go-gitea#26800) Updating the js libraries to latest version. (go-gitea#26795) Fix some slice append usages (go-gitea#26778) Use Go 1.21 for golangci-lint (go-gitea#26786) Fix notification circle (border-radius) (go-gitea#26794) Fix context filter has no effect in dashboard (go-gitea#26695) Add default label in branch select list (go-gitea#26697) Remove redundant nil check in `WalkGitLog` (go-gitea#26773) Remove fomantic `item` module (go-gitea#26775) Update info regarding internet connection for build (go-gitea#26776) Fix being unable to use a repo that prohibits accepting PRs as a PR source. (go-gitea#26785)

@CaiCandong

Backport #26664 by @CaiCandong > ### Description > If a new branch is pushed, and the repository has a rule that would require signed commits for the new branch, the commit is rejected with a 500 error regardless of whether it's signed. > > When pushing a new branch, the "old" commit is the empty ID (0000000000000000000000000000000000000000). verifyCommits has no provision for this and passes an invalid commit range to git rev-list. Prior to 1.19 this wasn't an issue because only pre-existing individual branches could be protected. > > I was able to reproduce with [try.gitea.io/CraigTest/test](https://try.gitea.io/CraigTest/test), which is set up with a blanket rule to require commits on all branches. Fix #25565 Very thanks to @Craig-Holmquist-NTI for reporting the bug and suggesting an valid solution! Co-authored-by: CaiCandong <50507092+CaiCandong@users.noreply.github.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [docker.io/gitea/gitea](https://github.com/go-gitea/gitea) | patch | `1.20.3` -> `1.20.4` | --- ### Release Notes <details> <summary>go-gitea/gitea (docker.io/gitea/gitea)</summary> ### [`v1.20.4`](https://github.com/go-gitea/gitea/blob/HEAD/CHANGELOG.md#1204---2023-09-08) [Compare Source](go-gitea/gitea@v1.20.3...v1.20.4) - SECURITY - Check blocklist for emails when adding them to account ([#26812](go-gitea/gitea#26812)) ([#26831](go-gitea/gitea#26831)) - ENHANCEMENTS - Add `branch_filter` to hooks API endpoints ([#26599](go-gitea/gitea#26599)) ([#26632](go-gitea/gitea#26632)) - Fix incorrect "tabindex" attributes ([#26733](go-gitea/gitea#26733)) ([#26734](go-gitea/gitea#26734)) - Use line-height: normal by default ([#26635](go-gitea/gitea#26635)) ([#26708](go-gitea/gitea#26708)) - Fix unable to display individual-level project ([#26198](go-gitea/gitea#26198)) ([#26636](go-gitea/gitea#26636)) - BUGFIXES - Fix wrong review requested number ([#26784](go-gitea/gitea#26784)) ([#26880](go-gitea/gitea#26880)) - Avoid double-unescaping of form value ([#26853](go-gitea/gitea#26853)) ([#26863](go-gitea/gitea#26863)) - Redirect from `{repo}/issues/new` to `{repo}/issues/new/choose` when blank issues are disabled ([#26813](go-gitea/gitea#26813)) ([#26847](go-gitea/gitea#26847)) - Sync tags when adopting repos ([#26816](go-gitea/gitea#26816)) ([#26834](go-gitea/gitea#26834)) - Fix verifyCommits error when push a new branch ([#26664](go-gitea/gitea#26664)) ([#26810](go-gitea/gitea#26810)) - Include the GITHUB_TOKEN/GITEA_TOKEN secret for fork pull requests ([#26759](go-gitea/gitea#26759)) ([#26806](go-gitea/gitea#26806)) - Fix some slice append usages ([#26778](go-gitea/gitea#26778)) ([#26798](go-gitea/gitea#26798)) - Add fix incorrect can_create_org_repo for org owner team ([#26683](go-gitea/gitea#26683)) ([#26791](go-gitea/gitea#26791)) - Fix bug for ctx usage ([#26763](go-gitea/gitea#26763)) - Make issue template field template access correct template data ([#26698](go-gitea/gitea#26698)) ([#26709](go-gitea/gitea#26709)) - Use correct minio error ([#26634](go-gitea/gitea#26634)) ([#26639](go-gitea/gitea#26639)) - Ignore the trailing slashes when comparing oauth2 redirect_uri ([#26597](go-gitea/gitea#26597)) ([#26618](go-gitea/gitea#26618)) - Set errwriter for urfave/cli v1 ([#26616](go-gitea/gitea#26616)) - Fix reopen logic for agit flow pull request ([#26399](go-gitea/gitea#26399)) ([#26613](go-gitea/gitea#26613)) - Fix context filter has no effect in dashboard ([#26695](go-gitea/gitea#26695)) ([#26811](go-gitea/gitea#26811)) - Fix being unable to use a repo that prohibits accepting PRs as a PR source. ([#26785](go-gitea/gitea#26785)) ([#26790](go-gitea/gitea#26790)) - Fix Page Not Found error ([#26768](go-gitea/gitea#26768)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.home/nrdufour/home-ops/pulls/79 Co-authored-by: Renovate <renovate@ptinem.io> Co-committed-by: Renovate <renovate@ptinem.io>

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 22, 2023

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 22, 2023

lunny added type/bug backport/v1.20 This PR should be backported to Gitea 1.20 labels Aug 22, 2023

lunny added this to the 1.21.0 milestone Aug 22, 2023

wxiaoguang reviewed Aug 22, 2023

View reviewed changes

routers/private/hook_verification.go Show resolved Hide resolved

pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 23, 2023

CaiCandong and others added 3 commits August 24, 2023 09:01

fix verifyCommits error when push a new branch

b4f1c36

add some comments

ff50ea9

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

Add test

de982aa

lunny force-pushed the bugfix/fix-verifyCommits branch from 56c1ef9 to de982aa Compare August 24, 2023 01:02

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 24, 2023

Merge branch 'main' into bugfix/fix-verifyCommits

0123201

CaiCandong marked this pull request as draft August 25, 2023 02:12

CaiCandong added 3 commits August 25, 2023 11:00

mock database

98eab3a

mock repo

4191603

add test

53cc3d5

pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 25, 2023

CaiCandong marked this pull request as ready for review August 25, 2023 03:07

fix lint

2a5c779

remove personal info

07a054a

CaiCandong marked this pull request as ready for review August 27, 2023 15:41

lunny approved these changes Aug 28, 2023

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 28, 2023

rm unnecessary file

dc24b94

lunny requested a review from wxiaoguang August 28, 2023 01:47

wxiaoguang added 2 commits August 29, 2023 08:43

Update repo1_hook_verification_dummy_gpg_key.txt

88bc2a7

Update repo1_hook_verification_dummy_gpg_key.txt

5812248

wxiaoguang approved these changes Aug 29, 2023

View reviewed changes

silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 29, 2023

silverwind enabled auto-merge (squash) August 29, 2023 23:26

Merge branch 'main' into bugfix/fix-verifyCommits

b24a457

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 29, 2023

GiteaBot added 2 commits August 30, 2023 09:40

Merge branch 'main' into bugfix/fix-verifyCommits

ef98bac

Merge branch 'main' into bugfix/fix-verifyCommits

92690f4

silverwind merged commit 815d267 into go-gitea:main Aug 30, 2023
23 checks passed

GiteaBot mentioned this pull request Aug 30, 2023

Fix verifyCommits error when push a new branch (#26664) #26810

Merged

GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Aug 30, 2023

CaiCandong deleted the bugfix/fix-verifyCommits branch September 4, 2023 12:41

go-gitea locked as resolved and limited conversation to collaborators Nov 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix verifyCommits error when push a new branch #26664

Fix verifyCommits error when push a new branch #26664

CaiCandong commented Aug 22, 2023

lunny commented Aug 22, 2023

CaiCandong commented Aug 22, 2023

lunny commented Aug 22, 2023

CaiCandong commented Aug 23, 2023

wxiaoguang commented Aug 23, 2023

lunny commented Aug 24, 2023

CaiCandong commented Aug 24, 2023 •

edited

Loading

lunny commented Aug 28, 2023

CaiCandong commented Aug 28, 2023

Fix verifyCommits error when push a new branch #26664

Fix verifyCommits error when push a new branch #26664

Conversation

CaiCandong commented Aug 22, 2023

Description

lunny commented Aug 22, 2023

CaiCandong commented Aug 22, 2023

lunny commented Aug 22, 2023

CaiCandong commented Aug 23, 2023

wxiaoguang commented Aug 23, 2023

lunny commented Aug 24, 2023

CaiCandong commented Aug 24, 2023 • edited Loading

lunny commented Aug 28, 2023

CaiCandong commented Aug 28, 2023

CaiCandong commented Aug 24, 2023 •

edited

Loading