From c071bdaf9662c87ce9def75b6f661700c62e375d Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Wed, 5 Jun 2024 00:05:10 +0000 Subject: [PATCH 01/13] Update elasticsearch Docker tag to v7.17.21 --- .forgejo/workflows/testing.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml index 8668c638ac..4b42ae455d 100644 --- a/.forgejo/workflows/testing.yml +++ b/.forgejo/workflows/testing.yml @@ -46,7 +46,7 @@ jobs: image: 'docker.io/node:20-bookworm' services: elasticsearch: - image: elasticsearch:7.5.0 + image: elasticsearch:7.17.21 env: discovery.type: single-node minio: From 6c4855e1eb425f86a15a130cb7ada1badf66fc11 Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Wed, 5 Jun 2024 00:05:27 +0000 Subject: [PATCH 02/13] Update module github.com/rhysd/actionlint/cmd/actionlint to v1.7.1 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index db30b91a61..90fd24d4b7 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,7 @@ SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.31.0 # renova XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 # renovate: datasource=go GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 # renovate: datasource=go -ACTIONLINT_PACKAGE ?= github.com/rhysd/actionlint/cmd/actionlint@v1.6.27 # renovate: datasource=go +ACTIONLINT_PACKAGE ?= github.com/rhysd/actionlint/cmd/actionlint@v1.7.1 # renovate: datasource=go DEADCODE_PACKAGE ?= golang.org/x/tools/internal/cmd/deadcode@v0.14.0 # renovate: datasource=go GOMOCK_PACKAGE ?= go.uber.org/mock/mockgen@v0.4.0 # renovate: datasource=go From b3bcae8bd6fa7f6bf5087f4952bbeb02cefd57c0 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 5 Jun 2024 07:24:09 +0200 Subject: [PATCH 03/13] Update dependency go to v1.22 There is no need to pin the patch release for the build environment. They are backward compatible and it prevents security upgrades to be taken into account. --- go.mod | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index bf9e25cca6..fbc4bcf8e0 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,8 @@ module code.gitea.io/gitea -go 1.22.3 +go 1.22.0 + +toolchain go1.22.3 require ( code.forgejo.org/forgejo/reply v1.0.1 From 1e2d51eb77b1c8f31efd479549679cad3905d8c0 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 5 Jun 2024 08:45:17 +0200 Subject: [PATCH 04/13] chore(dependency): remove GitHub specific actionlint dependency Forgejo has no GitHub workflows. The actionlint CLI is not flexible enough to be used for the validation of Forgejo Actions. --- Makefile | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Makefile b/Makefile index 90fd24d4b7..28ce44c72c 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,6 @@ SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.31.0 # renova XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 # renovate: datasource=go GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 # renovate: datasource=go -ACTIONLINT_PACKAGE ?= github.com/rhysd/actionlint/cmd/actionlint@v1.7.1 # renovate: datasource=go DEADCODE_PACKAGE ?= golang.org/x/tools/internal/cmd/deadcode@v0.14.0 # renovate: datasource=go GOMOCK_PACKAGE ?= go.uber.org/mock/mockgen@v0.4.0 # renovate: datasource=go @@ -219,7 +218,6 @@ help: @echo " - deps-py install python dependencies" @echo " - lint lint everything" @echo " - lint-fix lint everything and fix issues" - @echo " - lint-actions lint action workflow files" @echo " - lint-frontend lint frontend files" @echo " - lint-frontend-fix lint frontend files and fix issues" @echo " - lint-backend lint backend files" @@ -472,10 +470,6 @@ lint-go-vet: lint-editorconfig: $(GO) run $(EDITORCONFIG_CHECKER_PACKAGE) templates .forgejo/workflows -.PHONY: lint-actions -lint-actions: - $(GO) run $(ACTIONLINT_PACKAGE) - .PHONY: lint-templates lint-templates: .venv node_modules @node tools/lint-templates-svg.js @@ -882,7 +876,6 @@ deps-tools: $(GO) install $(XGO_PACKAGE) $(GO) install $(GO_LICENSES_PACKAGE) $(GO) install $(GOVULNCHECK_PACKAGE) - $(GO) install $(ACTIONLINT_PACKAGE) $(GO) install $(GOMOCK_PACKAGE) node_modules: package-lock.json From d2c4d833f4e0ef62a095f9829f927667e9dcca7e Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 5 Jun 2024 09:10:42 +0200 Subject: [PATCH 05/13] test(avatar): deleting a user avatar is idempotent If the avatar file in storage does not exist, it is not an error and the database can be updated. See 1be797faba301503e29db9de7eb32335a684464c Fix bug on avatar --- modules/storage/helper.go | 16 ++++----- modules/storage/helper_test.go | 4 +-- modules/storage/storage.go | 10 +++--- services/user/avatar_test.go | 61 ++++++++++++++++++++++++++-------- 4 files changed, 62 insertions(+), 29 deletions(-) diff --git a/modules/storage/helper.go b/modules/storage/helper.go index 4c24209f4f..95f1c7b9a8 100644 --- a/modules/storage/helper.go +++ b/modules/storage/helper.go @@ -10,30 +10,30 @@ import ( "os" ) -var UninitializedStorage = discardStorage("uninitialized storage") +var UninitializedStorage = DiscardStorage("uninitialized storage") -type discardStorage string +type DiscardStorage string -func (s discardStorage) Open(_ string) (Object, error) { +func (s DiscardStorage) Open(_ string) (Object, error) { return nil, fmt.Errorf("%s", s) } -func (s discardStorage) Save(_ string, _ io.Reader, _ int64) (int64, error) { +func (s DiscardStorage) Save(_ string, _ io.Reader, _ int64) (int64, error) { return 0, fmt.Errorf("%s", s) } -func (s discardStorage) Stat(_ string) (os.FileInfo, error) { +func (s DiscardStorage) Stat(_ string) (os.FileInfo, error) { return nil, fmt.Errorf("%s", s) } -func (s discardStorage) Delete(_ string) error { +func (s DiscardStorage) Delete(_ string) error { return fmt.Errorf("%s", s) } -func (s discardStorage) URL(_, _ string) (*url.URL, error) { +func (s DiscardStorage) URL(_, _ string) (*url.URL, error) { return nil, fmt.Errorf("%s", s) } -func (s discardStorage) IterateObjects(_ string, _ func(string, Object) error) error { +func (s DiscardStorage) IterateObjects(_ string, _ func(string, Object) error) error { return fmt.Errorf("%s", s) } diff --git a/modules/storage/helper_test.go b/modules/storage/helper_test.go index d0665b6dc9..f1f9791044 100644 --- a/modules/storage/helper_test.go +++ b/modules/storage/helper_test.go @@ -11,9 +11,9 @@ import ( ) func Test_discardStorage(t *testing.T) { - tests := []discardStorage{ + tests := []DiscardStorage{ UninitializedStorage, - discardStorage("empty"), + DiscardStorage("empty"), } for _, tt := range tests { t.Run(string(tt), func(t *testing.T) { diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 90f74eb2bd..b83b1c7929 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -170,7 +170,7 @@ func initAvatars() (err error) { func initAttachments() (err error) { if !setting.Attachment.Enabled { - Attachments = discardStorage("Attachment isn't enabled") + Attachments = DiscardStorage("Attachment isn't enabled") return nil } log.Info("Initialising Attachment storage with type: %s", setting.Attachment.Storage.Type) @@ -180,7 +180,7 @@ func initAttachments() (err error) { func initLFS() (err error) { if !setting.LFS.StartServer { - LFS = discardStorage("LFS isn't enabled") + LFS = DiscardStorage("LFS isn't enabled") return nil } log.Info("Initialising LFS storage with type: %s", setting.LFS.Storage.Type) @@ -202,7 +202,7 @@ func initRepoArchives() (err error) { func initPackages() (err error) { if !setting.Packages.Enabled { - Packages = discardStorage("Packages isn't enabled") + Packages = DiscardStorage("Packages isn't enabled") return nil } log.Info("Initialising Packages storage with type: %s", setting.Packages.Storage.Type) @@ -212,8 +212,8 @@ func initPackages() (err error) { func initActions() (err error) { if !setting.Actions.Enabled { - Actions = discardStorage("Actions isn't enabled") - ActionsArtifacts = discardStorage("ActionsArtifacts isn't enabled") + Actions = DiscardStorage("Actions isn't enabled") + ActionsArtifacts = DiscardStorage("ActionsArtifacts isn't enabled") return nil } log.Info("Initialising Actions storage with type: %s", setting.Actions.LogStorage.Type) diff --git a/services/user/avatar_test.go b/services/user/avatar_test.go index 0dc4dec651..557ddccec0 100644 --- a/services/user/avatar_test.go +++ b/services/user/avatar_test.go @@ -7,6 +7,7 @@ import ( "bytes" "image" "image/png" + "os" "testing" "code.gitea.io/gitea/models/db" @@ -18,30 +19,62 @@ import ( "github.com/stretchr/testify/assert" ) +type alreadyDeletedStorage struct { + storage.DiscardStorage +} + +func (s alreadyDeletedStorage) Delete(_ string) error { + return os.ErrNotExist +} + func TestUserDeleteAvatar(t *testing.T) { myImage := image.NewRGBA(image.Rect(0, 0, 1, 1)) var buff bytes.Buffer png.Encode(&buff, myImage) - assert.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - - err := UploadAvatar(db.DefaultContext, user, buff.Bytes()) - assert.NoError(t, err) - verification := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - assert.NotEqual(t, "", verification.Avatar) - t.Run("AtomicStorageFailure", func(t *testing.T) { - defer test.MockVariableValue[storage.ObjectStorage](&storage.Avatars, storage.UninitializedStorage)() + defer test.MockProtect[storage.ObjectStorage](&storage.Avatars)() + + assert.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + err := UploadAvatar(db.DefaultContext, user, buff.Bytes()) + assert.NoError(t, err) + verification := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.NotEqual(t, "", verification.Avatar) + + // fail to delete ... + storage.Avatars = storage.UninitializedStorage err = DeleteAvatar(db.DefaultContext, user) assert.Error(t, err) - verification := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + // ... the avatar is not removed from the database + verification = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) assert.True(t, verification.UseCustomAvatar) + + // already deleted ... + storage.Avatars = alreadyDeletedStorage{} + err = DeleteAvatar(db.DefaultContext, user) + assert.NoError(t, err) + + // ... the avatar is removed from the database + verification = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.Equal(t, "", verification.Avatar) }) - err = DeleteAvatar(db.DefaultContext, user) - assert.NoError(t, err) + t.Run("Success", func(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - verification = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - assert.Equal(t, "", verification.Avatar) + err := UploadAvatar(db.DefaultContext, user, buff.Bytes()) + assert.NoError(t, err) + verification := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.NotEqual(t, "", verification.Avatar) + + err = DeleteAvatar(db.DefaultContext, user) + assert.NoError(t, err) + + verification = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.Equal(t, "", verification.Avatar) + }) } From 4a3197fbc5d44a95b32f2070aaa4318e594b8b90 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 5 Jun 2024 08:55:10 +0200 Subject: [PATCH 06/13] chore(renovate): optimize config --- renovate.json | 53 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/renovate.json b/renovate.json index bff52598e6..ea61e4411b 100644 --- a/renovate.json +++ b/renovate.json @@ -28,7 +28,8 @@ "python", "golang", "docker.io/golang", - "docker.io/library/golang" + "docker.io/library/golang", + "mcr.microsoft.com/devcontainers/go" ], "matchUpdateTypes": ["minor"], "dependencyDashboardApproval": true @@ -74,7 +75,16 @@ }, { "description": "Split minor and patch updates", - "matchDepNames": ["vue", "github.com/urfave/cli/v2", "swagger-ui-dist"], + "matchDepNames": [ + "docker.io/golang", + "docker.io/library/golang", + "github.com/urfave/cli/v2", + "go", + "golang", + "python", + "swagger-ui-dist", + "vue" + ], "separateMinorPatch": true }, { @@ -89,6 +99,13 @@ "matchDepNames": ["ghcr.io/visualon/renovate"], "prPriority": 10 }, + { + "description": "Update go patch with higher prio to come through rate limit", + "matchDepNames": ["go", "golang", "docker.io/golang", "docker.io/library/golang"], + "matchUpdateTypes": ["patch"], + "prPriority": 10, + "schedule": ["at any time"] + }, { "description": "Disable actions/cascading-pr for now ", "matchDepNames": ["actions/cascading-pr"], @@ -99,23 +116,23 @@ "description": "Automerge some packages when CI succeeds", "extends": ["packages:linters", "packages:test"], "matchDepNames": [ - "github.com/golangci/golangci-lint/cmd/golangci-lint", - "github.com/go-testfixtures/testfixtures", - "github.com/PuerkitoBio/goquery", - "happy-dom", - "markdownlint-cli", - "updates", - "vite-string-plugin", - "@vue/test-utils" + "github.com/golangci/golangci-lint/cmd/golangci-lint", + "github.com/go-testfixtures/testfixtures", + "github.com/PuerkitoBio/goquery", + "happy-dom", + "markdownlint-cli", + "updates", + "vite-string-plugin", + "@vue/test-utils" ], "matchPackagePrefixes": [ - "@eslint-community/", - "@playwright/", - "@stoplight/spectral-cli", - "@stylistic/", - "ghcr.io/devcontainers/features/", - "ghcr.io/devcontainers-contrib/features/", - "mcr.microsoft.com/devcontainers/" + "@eslint-community/", + "@playwright/", + "@stoplight/spectral-cli", + "@stylistic/", + "ghcr.io/devcontainers/features/", + "ghcr.io/devcontainers-contrib/features/", + "mcr.microsoft.com/devcontainers/" ], "automerge": true }, @@ -126,7 +143,7 @@ }, { "description": "disallow `eslint-plugin-no-use-extend-native` v0.6.0+, requires eslint v9", - "matchDepNames":["eslint-plugin-no-use-extend-native"], + "matchDepNames": ["eslint-plugin-no-use-extend-native"], "allowedVersions": "<0.6.0" } ], From 32c882af91cf0c0f68a486876680b38095fa53a0 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 5 Jun 2024 12:33:10 +0200 Subject: [PATCH 07/13] test(oauth): coverage for the redirection of a denied grant See 886a675f62233dcde3ac0d7b2181484f29344f26 Return `access_denied` error when an OAuth2 request is denied --- release-notes/8.0.0/fix/4026.md | 1 + tests/integration/oauth_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 release-notes/8.0.0/fix/4026.md diff --git a/release-notes/8.0.0/fix/4026.md b/release-notes/8.0.0/fix/4026.md new file mode 100644 index 0000000000..747c3a789e --- /dev/null +++ b/release-notes/8.0.0/fix/4026.md @@ -0,0 +1 @@ +- when an OAuth grant request submitted to a Forgejo user is denied, the server from which the request originates is not notified that it has been denied diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 46beddb5f3..4d51588ec6 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -608,3 +608,24 @@ func TestSignUpViaOAuthWithMissingFields(t *testing.T) { resp := MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, test.RedirectURL(resp), "/user/link_account") } + +func TestOAuth_GrantApplicationOAuth(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + req := NewRequest(t, "GET", "/login/oauth/authorize?client_id=da7da3ba-9a13-4167-856f-3899de0b0138&redirect_uri=a&response_type=code&state=thestate") + ctx := loginUser(t, "user4") + resp := ctx.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + htmlDoc.AssertElement(t, "#authorize-app", true) + + req = NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "client_id": "da7da3ba-9a13-4167-856f-3899de0b0138", + "redirect_uri": "a", + "state": "thestate", + "granted": "false", + }) + resp = ctx.MakeRequest(t, req, http.StatusSeeOther) + assert.Contains(t, test.RedirectURL(resp), "error=access_denied&error_description=the+request+is+denied") +} From caadd1815a0343ae9c070e7befb6b40cf868c18e Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 5 Jun 2024 15:42:50 +0200 Subject: [PATCH 08/13] fix(oauth): HTML snippets in templates can be displayed These changes were missed when cherry-picking the following c9d0e63c202827756c637d9ca7bbde685c1984b7 Remove unnecessary "Str2html" modifier from templates (#29319) Fixes: https://codeberg.org/forgejo/forgejo/issues/3623 --- routers/web/auth/oauth.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 72473701de..c33c8029ce 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "html" + "html/template" "io" "net/http" "net/url" @@ -502,11 +503,11 @@ func AuthorizeOAuth(ctx *context.Context) { ctx.Data["Scope"] = form.Scope ctx.Data["Nonce"] = form.Nonce if user != nil { - ctx.Data["ApplicationCreatorLinkHTML"] = fmt.Sprintf(`@%s`, html.EscapeString(user.HomeLink()), html.EscapeString(user.Name)) + ctx.Data["ApplicationCreatorLinkHTML"] = template.HTML(fmt.Sprintf(`@%s`, html.EscapeString(user.HomeLink()), html.EscapeString(user.Name))) } else { - ctx.Data["ApplicationCreatorLinkHTML"] = fmt.Sprintf(`%s`, html.EscapeString(setting.AppSubURL+"/"), html.EscapeString(setting.AppName)) + ctx.Data["ApplicationCreatorLinkHTML"] = template.HTML(fmt.Sprintf(`%s`, html.EscapeString(setting.AppSubURL+"/"), html.EscapeString(setting.AppName))) } - ctx.Data["ApplicationRedirectDomainHTML"] = "" + html.EscapeString(form.RedirectURI) + "" + ctx.Data["ApplicationRedirectDomainHTML"] = template.HTML("" + html.EscapeString(form.RedirectURI) + "") // TODO document SESSION <=> FORM err = ctx.Session.Set("client_id", app.ClientID) if err != nil { From 1013da463f4c84c55ba4f5672c916c0763aa89da Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 5 Jun 2024 15:17:11 +0200 Subject: [PATCH 09/13] test: webhook open project expected fields --- services/webhook/default_test.go | 36 ++++++++++++++++++++++++++++++++ services/webhook/general_test.go | 11 ++++++++++ 2 files changed, 47 insertions(+) diff --git a/services/webhook/default_test.go b/services/webhook/default_test.go index 29f1521cca..e6e59fed03 100644 --- a/services/webhook/default_test.go +++ b/services/webhook/default_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/json" webhook_module "code.gitea.io/gitea/modules/webhook" + jsoniter "github.com/json-iterator/go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -222,3 +223,38 @@ func TestForgejoPayload(t *testing.T) { assert.Equal(t, "refs/heads/test", body.Ref) // full ref }) } + +func TestOpenProjectPayload(t *testing.T) { + t.Run("PullRequest", func(t *testing.T) { + p := pullRequestTestPayload() + data, err := p.JSONPayload() + require.NoError(t, err) + + // adapted from https://github.com/opf/openproject/blob/4c5c45fe995da0060902bc8dd5f1bf704d0b8737/modules/github_integration/lib/open_project/github_integration/services/upsert_pull_request.rb#L56 + j := jsoniter.Get(data, "pull_request") + + assert.Equal(t, 12, j.Get("id").MustBeValid().ToInt()) + assert.Equal(t, "user1", j.Get("user", "login").MustBeValid().ToString()) + assert.Equal(t, 12, j.Get("number").MustBeValid().ToInt()) + assert.Equal(t, "http://localhost:3000/test/repo/pulls/12", j.Get("html_url").MustBeValid().ToString()) + assert.Equal(t, jsoniter.NilValue, j.Get("updated_at").ValueType()) + assert.Equal(t, "", j.Get("state").MustBeValid().ToString()) + assert.Equal(t, "Fix bug", j.Get("title").MustBeValid().ToString()) + assert.Equal(t, "fixes bug #2", j.Get("body").MustBeValid().ToString()) + + assert.Equal(t, "test/repo", j.Get("base", "repo", "full_name").MustBeValid().ToString()) + assert.Equal(t, "http://localhost:3000/test/repo", j.Get("base", "repo", "html_url").MustBeValid().ToString()) + + assert.Equal(t, false, j.Get("draft").MustBeValid().ToBool()) + assert.Equal(t, jsoniter.NilValue, j.Get("merge_commit_sha").ValueType()) + assert.Equal(t, false, j.Get("merged").MustBeValid().ToBool()) + assert.Equal(t, jsoniter.NilValue, j.Get("merged_by").ValueType()) + assert.Equal(t, jsoniter.NilValue, j.Get("merged_at").ValueType()) + assert.Equal(t, 0, j.Get("comments").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("review_comments").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("additions").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("deletions").MustBeValid().ToInt()) + assert.Equal(t, 0, j.Get("changed_files").MustBeValid().ToInt()) + // assert.Equal(t,"labels:", j.Get("labels").map { |values| extract_label_values(values) ) + }) +} diff --git a/services/webhook/general_test.go b/services/webhook/general_test.go index 41bac3fd04..5b9bfdc1b2 100644 --- a/services/webhook/general_test.go +++ b/services/webhook/general_test.go @@ -281,6 +281,17 @@ func pullRequestTestPayload() *api.PullRequestPayload { Title: "Milestone Title", Description: "Milestone Description", }, + Base: &api.PRBranchInfo{ + Name: "branch1", + Ref: "refs/pull/2/head", + Sha: "4a357436d925b5c974181ff12a994538ddc5a269", + RepoID: 1, + Repository: &api.Repository{ + HTMLURL: "http://localhost:3000/test/repo", + Name: "repo", + FullName: "test/repo", + }, + }, }, Review: &api.ReviewPayload{ Content: "good job", From fb7b17d24081535212e7e06e6fcfcaa6327dbc82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Rosenhammer?= Date: Sun, 26 May 2024 06:08:13 +0200 Subject: [PATCH 10/13] Make gitea webhooks openproject compatible (gitea#28435) This PR adds some fields to the gitea webhook payload that [openproject](https://www.openproject.org/) expects to exists in order to process the webhooks. These fields do exists in Github's webhook payload so adding them makes Gitea's native webhook more compatible towards Github's. --- models/issues/pull.go | 15 ++++++++ modules/structs/issue.go | 1 + modules/structs/pull.go | 6 ++++ modules/structs/user.go | 2 ++ services/convert/issue.go | 2 ++ services/convert/pull.go | 64 ++++++++++++++++++++++------------ services/convert/user.go | 1 + templates/swagger/v1_json.tmpl | 34 ++++++++++++++++++ 8 files changed, 102 insertions(+), 23 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index ae7fb35eab..ef49a51045 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -431,6 +431,21 @@ func (pr *PullRequest) GetGitHeadBranchRefName() string { return fmt.Sprintf("%s%s", git.BranchPrefix, pr.HeadBranch) } +// GetReviewCommentsCount returns the number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR) +func (pr *PullRequest) GetReviewCommentsCount(ctx context.Context) int { + opts := FindCommentsOptions{ + Type: CommentTypeReview, + IssueID: pr.IssueID, + } + conds := opts.ToConds() + + count, err := db.GetEngine(ctx).Where(conds).Count(new(Comment)) + if err != nil { + return 0 + } + return int(count) +} + // IsChecking returns true if this pull request is still checking conflict. func (pr *PullRequest) IsChecking() bool { return pr.Status == PullRequestStatusChecking diff --git a/modules/structs/issue.go b/modules/structs/issue.go index e2b49e94c5..7ba7f77158 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -30,6 +30,7 @@ type PullRequestMeta struct { HasMerged bool `json:"merged"` Merged *time.Time `json:"merged_at"` IsWorkInProgress bool `json:"draft"` + HTMLURL string `json:"html_url"` } // RepositoryMeta basic repository information diff --git a/modules/structs/pull.go b/modules/structs/pull.go index b04def52b8..525d90c28e 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -21,8 +21,14 @@ type PullRequest struct { Assignees []*User `json:"assignees"` RequestedReviewers []*User `json:"requested_reviewers"` State StateType `json:"state"` + Draft bool `json:"draft"` IsLocked bool `json:"is_locked"` Comments int `json:"comments"` + // number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR) + ReviewComments int `json:"review_comments"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + ChangedFiles int `json:"changed_files"` HTMLURL string `json:"html_url"` DiffURL string `json:"diff_url"` diff --git a/modules/structs/user.go b/modules/structs/user.go index ad529c966e..be20349e53 100644 --- a/modules/structs/user.go +++ b/modules/structs/user.go @@ -27,6 +27,8 @@ type User struct { Email string `json:"email"` // URL to the user's avatar AvatarURL string `json:"avatar_url"` + // URL to the user's gitea page + HTMLURL string `json:"html_url"` // User locale Language string `json:"language"` // Is the user an administrator diff --git a/services/convert/issue.go b/services/convert/issue.go index 047feb8aa7..f514dc4313 100644 --- a/services/convert/issue.go +++ b/services/convert/issue.go @@ -107,6 +107,8 @@ func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Iss if issue.PullRequest.HasMerged { apiIssue.PullRequest.Merged = issue.PullRequest.MergedUnix.AsTimePtr() } + // Add pr's html url + apiIssue.PullRequest.HTMLURL = issue.HTMLURL() } } if issue.DeadlineUnix != 0 { diff --git a/services/convert/pull.go b/services/convert/pull.go index da75cb3db2..c214805ed5 100644 --- a/services/convert/pull.go +++ b/services/convert/pull.go @@ -61,29 +61,31 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } apiPullRequest := &api.PullRequest{ - ID: pr.ID, - URL: pr.Issue.HTMLURL(), - Index: pr.Index, - Poster: apiIssue.Poster, - Title: apiIssue.Title, - Body: apiIssue.Body, - Labels: apiIssue.Labels, - Milestone: apiIssue.Milestone, - Assignee: apiIssue.Assignee, - Assignees: apiIssue.Assignees, - State: apiIssue.State, - IsLocked: apiIssue.IsLocked, - Comments: apiIssue.Comments, - HTMLURL: pr.Issue.HTMLURL(), - DiffURL: pr.Issue.DiffURL(), - PatchURL: pr.Issue.PatchURL(), - HasMerged: pr.HasMerged, - MergeBase: pr.MergeBase, - Mergeable: pr.Mergeable(ctx), - Deadline: apiIssue.Deadline, - Created: pr.Issue.CreatedUnix.AsTimePtr(), - Updated: pr.Issue.UpdatedUnix.AsTimePtr(), - PinOrder: apiIssue.PinOrder, + ID: pr.ID, + URL: pr.Issue.HTMLURL(), + Index: pr.Index, + Poster: apiIssue.Poster, + Title: apiIssue.Title, + Body: apiIssue.Body, + Labels: apiIssue.Labels, + Milestone: apiIssue.Milestone, + Assignee: apiIssue.Assignee, + Assignees: apiIssue.Assignees, + State: apiIssue.State, + Draft: pr.IsWorkInProgress(ctx), + IsLocked: apiIssue.IsLocked, + Comments: apiIssue.Comments, + ReviewComments: pr.GetReviewCommentsCount(ctx), + HTMLURL: pr.Issue.HTMLURL(), + DiffURL: pr.Issue.DiffURL(), + PatchURL: pr.Issue.PatchURL(), + HasMerged: pr.HasMerged, + MergeBase: pr.MergeBase, + Mergeable: pr.Mergeable(ctx), + Deadline: apiIssue.Deadline, + Created: pr.Issue.CreatedUnix.AsTimePtr(), + Updated: pr.Issue.UpdatedUnix.AsTimePtr(), + PinOrder: apiIssue.PinOrder, AllowMaintainerEdit: pr.AllowMaintainerEdit, @@ -178,6 +180,12 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u return nil } + // Outer scope variables to be used in diff calculation + var ( + startCommitID string + endCommitID string + ) + if git.IsErrBranchNotExist(err) { headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref) if err != nil && !git.IsErrNotExist(err) { @@ -186,6 +194,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } if err == nil { apiPullRequest.Head.Sha = headCommitID + endCommitID = headCommitID } } else { commit, err := headBranch.GetCommit() @@ -196,8 +205,17 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u if err == nil { apiPullRequest.Head.Ref = pr.HeadBranch apiPullRequest.Head.Sha = commit.ID.String() + endCommitID = commit.ID.String() } } + + // Calculate diff + startCommitID = pr.MergeBase + + apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID) + if err != nil { + log.Error("GetDiffShortStat: %v", err) + } } if len(apiPullRequest.Head.Sha) == 0 && len(apiPullRequest.Head.Ref) != 0 { diff --git a/services/convert/user.go b/services/convert/user.go index 789bc51097..94a400de5d 100644 --- a/services/convert/user.go +++ b/services/convert/user.go @@ -53,6 +53,7 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap FullName: user.FullName, Email: user.GetPlaceholderEmail(), AvatarURL: user.AvatarLink(ctx), + HTMLURL: user.HTMLURL(), Created: user.CreatedUnix.AsTime(), Restricted: user.IsRestricted, Location: user.Location, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6a488559a0..8e9ee8b5cd 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -23413,6 +23413,11 @@ "description": "PullRequest represents a pull request", "type": "object", "properties": { + "additions": { + "type": "integer", + "format": "int64", + "x-go-name": "Additions" + }, "allow_maintainer_edit": { "type": "boolean", "x-go-name": "AllowMaintainerEdit" @@ -23434,6 +23439,11 @@ "type": "string", "x-go-name": "Body" }, + "changed_files": { + "type": "integer", + "format": "int64", + "x-go-name": "ChangedFiles" + }, "closed_at": { "type": "string", "format": "date-time", @@ -23449,10 +23459,19 @@ "format": "date-time", "x-go-name": "Created" }, + "deletions": { + "type": "integer", + "format": "int64", + "x-go-name": "Deletions" + }, "diff_url": { "type": "string", "x-go-name": "DiffURL" }, + "draft": { + "type": "boolean", + "x-go-name": "Draft" + }, "due_date": { "type": "string", "format": "date-time", @@ -23529,6 +23548,12 @@ }, "x-go-name": "RequestedReviewers" }, + "review_comments": { + "description": "number of review comments made on the diff of a PR review (not including comments on commits or issues in a PR)", + "type": "integer", + "format": "int64", + "x-go-name": "ReviewComments" + }, "state": { "$ref": "#/definitions/StateType" }, @@ -23559,6 +23584,10 @@ "type": "boolean", "x-go-name": "IsWorkInProgress" }, + "html_url": { + "type": "string", + "x-go-name": "HTMLURL" + }, "merged": { "type": "boolean", "x-go-name": "HasMerged" @@ -24904,6 +24933,11 @@ "type": "string", "x-go-name": "FullName" }, + "html_url": { + "description": "URL to the user's gitea page", + "type": "string", + "x-go-name": "HTMLURL" + }, "id": { "description": "the user's id", "type": "integer", From 8763225972945a42b0a08ff29c3e9e3ddbae4955 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 5 Jun 2024 15:21:57 +0200 Subject: [PATCH 11/13] add release note --- release-notes/8.0.0/feat/4027.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 release-notes/8.0.0/feat/4027.md diff --git a/release-notes/8.0.0/feat/4027.md b/release-notes/8.0.0/feat/4027.md new file mode 100644 index 0000000000..bdae695679 --- /dev/null +++ b/release-notes/8.0.0/feat/4027.md @@ -0,0 +1 @@ +- Gitea/Forgejo webhook payload include additional fields (`html_url`, `additions`, `deletions`, `review_comments`...) for better compatbility with [OpenProject](https://www.openproject.org/), ported from [gitea#28435](https://github.com/go-gitea/gitea/pull/28435). From 8dd72661aff626f2294185bf21f51483bd8d1070 Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Wed, 5 Jun 2024 16:08:45 +0000 Subject: [PATCH 12/13] Update ghcr.io/visualon/renovate Docker tag to v37.391.2 --- .forgejo/workflows/renovate.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.forgejo/workflows/renovate.yml b/.forgejo/workflows/renovate.yml index 35ff06fa43..bd275253b8 100644 --- a/.forgejo/workflows/renovate.yml +++ b/.forgejo/workflows/renovate.yml @@ -22,7 +22,7 @@ jobs: runs-on: docker container: - image: ghcr.io/visualon/renovate:37.385.0 + image: ghcr.io/visualon/renovate:37.391.2 steps: - name: Load renovate repo cache From 3bfec270acde189fe5e5e8f2e65be9e5a1be61d9 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 5 Jun 2024 22:07:40 +0200 Subject: [PATCH 13/13] chore(dependency): whitelist mholt/archiver/v3 CVE-2024-0406 It is not possible to tell vulncheck that Forgejo is not affected by CVE-2024-0406. Use a mirror of the repository to do that. Refs: https://github.com/mholt/archiver/issues/404 --- go.mod | 2 ++ go.sum | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index fbc4bcf8e0..25cdd1be19 100644 --- a/go.mod +++ b/go.mod @@ -312,3 +312,5 @@ exclude github.com/gofrs/uuid v4.0.0+incompatible exclude github.com/goccy/go-json v0.4.11 exclude github.com/satori/go.uuid v1.2.0 + +replace github.com/mholt/archiver/v3 => code.forgejo.org/forgejo/archiver/v3 v3.5.1 diff --git a/go.sum b/go.sum index 173cf51db0..170f704f3d 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ cloud.google.com/go/compute v1.23.3 h1:6sVlXXBmbd7jNX0Ipq0trII3e4n1/MsADLK6a+aiV cloud.google.com/go/compute v1.23.3/go.mod h1:VCgBUoMnIVIR0CscqQiPJLAG25E3ZRZMzcFZeQ+h8CI= cloud.google.com/go/compute/metadata v0.2.3 h1:mg4jlk7mCAj6xXp9UJ4fjI9VUI5rubuGBW5aJ7UnBMY= cloud.google.com/go/compute/metadata v0.2.3/go.mod h1:VAV5nSsACxMJvgaAuX6Pk2AawlZn8kiOGuCv6gTkwuA= +code.forgejo.org/forgejo/archiver/v3 v3.5.1 h1:UmmbA7D5550uf71SQjarmrn6yKwOGxtEjb3jaYYtmSE= +code.forgejo.org/forgejo/archiver/v3 v3.5.1/go.mod h1:e3dqJ7H78uzsRSEACH1joayhuSyhnonssnDhppzS1L4= code.forgejo.org/forgejo/reply v1.0.1 h1:usZi5yx7/g0D+xtGPJEM6mCvoDNdWvmtJu5J9/B/KBI= code.forgejo.org/forgejo/reply v1.0.1/go.mod h1:RyZUfzQLc+fuLIGjTSQWDAJWPiL4WtKXB/FifT5fM7U= code.gitea.io/actions-proto-go v0.4.0 h1:OsPBPhodXuQnsspG1sQ4eRE1PeoZyofd7+i73zCwnsU= @@ -515,8 +517,6 @@ github.com/meilisearch/meilisearch-go v0.26.1 h1:3bmo2uLijX7kvBmiZ9LupVfC95TFcRJ github.com/meilisearch/meilisearch-go v0.26.1/go.mod h1:SxuSqDcPBIykjWz1PX+KzsYzArNLSCadQodWs8extS0= github.com/mholt/acmez/v2 v2.0.1 h1:3/3N0u1pLjMK4sNEAFSI+bcvzbPhRpY383sy1kLHJ6k= github.com/mholt/acmez/v2 v2.0.1/go.mod h1:fX4c9r5jYwMyMsC+7tkYRxHibkOTgta5DIFGoe67e1U= -github.com/mholt/archiver/v3 v3.5.1 h1:rDjOBX9JSF5BvoJGvjqK479aL70qh9DIpZCl+k7Clwo= -github.com/mholt/archiver/v3 v3.5.1/go.mod h1:e3dqJ7H78uzsRSEACH1joayhuSyhnonssnDhppzS1L4= github.com/microcosm-cc/bluemonday v1.0.26 h1:xbqSvqzQMeEHCqMi64VAs4d8uy6Mequs3rQ0k/Khz58= github.com/microcosm-cc/bluemonday v1.0.26/go.mod h1:JyzOCs9gkyQyjs+6h10UEVSe02CGwkhd72Xdqh78TWs= github.com/miekg/dns v1.1.59 h1:C9EXc/UToRwKLhK5wKU/I4QVsBUc8kE6MkHBkeypWZs=