From 7d7007dca75ab3a3a48b9f3fd7cc4350cc626870 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Pawe=C5=82=20Bogus=C5=82awski?= Date: Thu, 11 Feb 2021 18:34:34 +0100 Subject: [PATCH] Added option to disable webhooks (#13176) * Added option to disable web hooks This mod introduces DISABLE_WEB_HOOKS parameter in [security] section of app.ini (by default set to false). If set to true it disables web hooks feature. Any existing undelivered web hook tasks will be cancelled. Any existing web hook definitions will be left untouched in db but its delivery tasks will be ignored. Author-Change-Id: IB#1105130 * Webhook spelling fixed Webhook spelling fixed. Fixes: 07df6614dc84cdd2e9f39c57577fa1062bd70012 Related: https://github.com/go-gitea/gitea/pull/13176#pullrequestreview-510868421 Author-Change-Id: IB#1105174 * Parameter description fixed Parameter description fixed. Fixes: 07df6614dc84cdd2e9f39c57577fa1062bd70012 Related: https://github.com/go-gitea/gitea/pull/13176#pullrequestreview-514086107 Author-Change-Id: IB#1105174 --- custom/conf/app.example.ini | 2 ++ .../doc/advanced/config-cheat-sheet.en-us.md | 1 + modules/setting/setting.go | 2 ++ modules/templates/helper.go | 3 ++ routers/api/v1/api.go | 30 ++++++++++++------- routers/routes/web.go | 26 ++++++++++------ services/webhook/deliver.go | 4 +++ services/webhook/webhook.go | 5 ++++ templates/admin/navbar.tmpl | 8 +++-- templates/org/settings/navbar.tmpl | 2 ++ templates/repo/settings/nav.tmpl | 2 ++ templates/repo/settings/navbar.tmpl | 8 +++-- 12 files changed, 68 insertions(+), 25 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index cec7e4255a..747173b5ae 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -556,6 +556,8 @@ IMPORT_LOCAL_PATHS = false ; It also enables them to access other resources available to the user on the operating system that is running the Gitea instance and perform arbitrary actions in the name of the Gitea OS user. ; WARNING: This maybe harmful to you website or your operating system. DISABLE_GIT_HOOKS = true +; Set to true to disable webhooks feature. +DISABLE_WEBHOOKS = false ; Set to false to allow pushes to gitea repositories despite having an incomplete environment - NOT RECOMMENDED ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET = true ;Comma separated list of character classes required to pass minimum complexity. diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 9b5c4cbf2d..b65f59ce0c 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -396,6 +396,7 @@ relation to port exhaustion. It also enables them to access other resources available to the user on the operating system that is running the Gitea instance and perform arbitrary actions in the name of the Gitea OS user. This maybe harmful to you website or your operating system. +- `DISABLE_WEBHOOKS`: **false**: Set to `true` to disable webhooks feature. - `ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET`: **true**: Set to `false` to allow local users to push to gitea-repositories without setting up the Gitea environment. This is not recommended and if you want local users to push to gitea repositories you should set the environment appropriately. - `IMPORT_LOCAL_PATHS`: **false**: Set to `false` to prevent all users (including admin) from importing local path on server. - `INTERNAL_TOKEN`: **\**: Secret used to validate communication within Gitea binary. diff --git a/modules/setting/setting.go b/modules/setting/setting.go index dd38f5be45..54ddb6937c 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -156,6 +156,7 @@ var ( MinPasswordLength int ImportLocalPaths bool DisableGitHooks bool + DisableWebhooks bool OnlyAllowPushIfGiteaEnvironmentSet bool PasswordComplexity []string PasswordHashAlgo string @@ -801,6 +802,7 @@ func NewContext() { MinPasswordLength = sec.Key("MIN_PASSWORD_LENGTH").MustInt(6) ImportLocalPaths = sec.Key("IMPORT_LOCAL_PATHS").MustBool(false) DisableGitHooks = sec.Key("DISABLE_GIT_HOOKS").MustBool(true) + DisableWebhooks = sec.Key("DISABLE_WEBHOOKS").MustBool(false) OnlyAllowPushIfGiteaEnvironmentSet = sec.Key("ONLY_ALLOW_PUSH_IF_GITEA_ENVIRONMENT_SET").MustBool(true) PasswordHashAlgo = sec.Key("PASSWORD_HASH_ALGO").MustString("argon2") CSRFCookieHTTPOnly = sec.Key("CSRF_COOKIE_HTTP_ONLY").MustBool(true) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index b8e4f5d505..00ccd49cb4 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -229,6 +229,9 @@ func NewFuncMap() []template.FuncMap { "DisableGitHooks": func() bool { return setting.DisableGitHooks }, + "DisableWebhooks": func() bool { + return setting.DisableWebhooks + }, "DisableImportLocal": func() bool { return !setting.ImportLocalPaths }, diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 85c4e4d5bf..855e44b65b 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -383,6 +383,16 @@ func reqGitHook() func(ctx *context.APIContext) { } } +// reqWebhooksEnabled requires webhooks to be enabled by admin. +func reqWebhooksEnabled() func(ctx *context.APIContext) { + return func(ctx *context.APIContext) { + if setting.DisableWebhooks { + ctx.Error(http.StatusForbidden, "", "webhooks disabled by administrator") + return + } + } +} + func orgAssignment(args ...bool) func(ctx *context.APIContext) { var ( assignOrg bool @@ -703,6 +713,14 @@ func Routes() *web.Route { m.Combo("/notifications"). Get(reqToken(), notify.ListRepoNotifications). Put(reqToken(), notify.ReadRepoNotifications) + m.Group("/hooks/git", func() { + m.Combo("").Get(repo.ListGitHooks) + m.Group("/{id}", func() { + m.Combo("").Get(repo.GetGitHook). + Patch(bind(api.EditGitHookOption{}), repo.EditGitHook). + Delete(repo.DeleteGitHook) + }) + }, reqToken(), reqAdmin(), reqGitHook(), context.ReferencesGitRepo(true)) m.Group("/hooks", func() { m.Combo("").Get(repo.ListHooks). Post(bind(api.CreateHookOption{}), repo.CreateHook) @@ -712,15 +730,7 @@ func Routes() *web.Route { Delete(repo.DeleteHook) m.Post("/tests", context.RepoRefForAPI, repo.TestHook) }) - m.Group("/git", func() { - m.Combo("").Get(repo.ListGitHooks) - m.Group("/{id}", func() { - m.Combo("").Get(repo.GetGitHook). - Patch(bind(api.EditGitHookOption{}), repo.EditGitHook). - Delete(repo.DeleteGitHook) - }) - }, reqGitHook(), context.ReferencesGitRepo(true)) - }, reqToken(), reqAdmin()) + }, reqToken(), reqAdmin(), reqWebhooksEnabled()) m.Group("/collaborators", func() { m.Get("", reqAnyRepoReader(), repo.ListCollaborators) m.Combo("/{collaborator}").Get(reqAnyRepoReader(), repo.IsCollaborator). @@ -984,7 +994,7 @@ func Routes() *web.Route { m.Combo("/{id}").Get(org.GetHook). Patch(bind(api.EditHookOption{}), org.EditHook). Delete(org.DeleteHook) - }, reqToken(), reqOrgOwnership()) + }, reqToken(), reqOrgOwnership(), reqWebhooksEnabled()) }, orgAssignment(true)) m.Group("/teams/{teamid}", func() { m.Combo("").Get(org.GetTeam). diff --git a/routers/routes/web.go b/routers/routes/web.go index 2f28e567f9..389e050376 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -248,6 +248,14 @@ func RegisterRoutes(m *web.Route) { } } + // webhooksEnabled requires webhooks to be enabled by admin. + webhooksEnabled := func(ctx *context.Context) { + if setting.DisableWebhooks { + ctx.Error(403) + return + } + } + // FIXME: not all routes need go through same middleware. // Especially some AJAX requests, we can reduce middleware number to improve performance. // Routers. @@ -446,7 +454,7 @@ func RegisterRoutes(m *web.Route) { m.Post("/matrix/{id}", bindIgnErr(auth.NewMatrixHookForm{}), repo.MatrixHooksEditPost) m.Post("/msteams/{id}", bindIgnErr(auth.NewMSTeamsHookForm{}), repo.MSTeamsHooksEditPost) m.Post("/feishu/{id}", bindIgnErr(auth.NewFeishuHookForm{}), repo.FeishuHooksEditPost) - }) + }, webhooksEnabled) m.Group("/{configType:default-hooks|system-hooks}", func() { m.Get("/{type}/new", repo.WebhooksNew) @@ -568,7 +576,7 @@ func RegisterRoutes(m *web.Route) { m.Post("/matrix/{id}", bindIgnErr(auth.NewMatrixHookForm{}), repo.MatrixHooksEditPost) m.Post("/msteams/{id}", bindIgnErr(auth.NewMSTeamsHookForm{}), repo.MSTeamsHooksEditPost) m.Post("/feishu/{id}", bindIgnErr(auth.NewFeishuHookForm{}), repo.FeishuHooksEditPost) - }) + }, webhooksEnabled) m.Group("/labels", func() { m.Get("", org.RetrieveLabels, org.Labels) @@ -621,6 +629,12 @@ func RegisterRoutes(m *web.Route) { Post(bindIgnErr(auth.ProtectBranchForm{}), context.RepoMustNotBeArchived(), repo.SettingsProtectedBranchPost) }, repo.MustBeNotEmpty) + m.Group("/hooks/git", func() { + m.Get("", repo.GitHooks) + m.Combo("/{name}").Get(repo.GitHooksEdit). + Post(repo.GitHooksEditPost) + }, context.GitHookService()) + m.Group("/hooks", func() { m.Get("", repo.Webhooks) m.Post("/delete", repo.DeleteWebhook) @@ -645,13 +659,7 @@ func RegisterRoutes(m *web.Route) { m.Post("/matrix/{id}", bindIgnErr(auth.NewMatrixHookForm{}), repo.MatrixHooksEditPost) m.Post("/msteams/{id}", bindIgnErr(auth.NewMSTeamsHookForm{}), repo.MSTeamsHooksEditPost) m.Post("/feishu/{id}", bindIgnErr(auth.NewFeishuHookForm{}), repo.FeishuHooksEditPost) - - m.Group("/git", func() { - m.Get("", repo.GitHooks) - m.Combo("/{name}").Get(repo.GitHooksEdit). - Post(repo.GitHooksEditPost) - }, context.GitHookService()) - }) + }, webhooksEnabled) m.Group("/keys", func() { m.Combo("").Get(repo.DeployKeys). diff --git a/services/webhook/deliver.go b/services/webhook/deliver.go index 44c1a18b6c..8ac7d8c192 100644 --- a/services/webhook/deliver.go +++ b/services/webhook/deliver.go @@ -141,6 +141,10 @@ func Deliver(t *models.HookTask) error { } }() + if setting.DisableWebhooks { + return fmt.Errorf("Webhook task skipped (webhooks disabled): [%d]", t.ID) + } + resp, err := webhookHTTPClient.Do(req) if err != nil { t.ResponseInfo.Body = fmt.Sprintf("Delivery: %v", err) diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index 7b6bc555f7..87f28a5cdc 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -120,6 +120,11 @@ func checkBranch(w *models.Webhook, branch string) bool { } func prepareWebhook(w *models.Webhook, repo *models.Repository, event models.HookEventType, p api.Payloader) error { + // Skip sending if webhooks are disabled. + if setting.DisableWebhooks { + return nil + } + for _, e := range w.EventCheckers() { if event == e.Type { if !e.Has() { diff --git a/templates/admin/navbar.tmpl b/templates/admin/navbar.tmpl index 953076d808..c656d0619b 100644 --- a/templates/admin/navbar.tmpl +++ b/templates/admin/navbar.tmpl @@ -12,9 +12,11 @@ {{.i18n.Tr "admin.repositories"}} - - {{.i18n.Tr "admin.hooks"}} - + {{if not DisableWebhooks}} + + {{.i18n.Tr "admin.hooks"}} + + {{end}} {{.i18n.Tr "admin.authentication"}} diff --git a/templates/org/settings/navbar.tmpl b/templates/org/settings/navbar.tmpl index 63114b056e..b4edbb077f 100644 --- a/templates/org/settings/navbar.tmpl +++ b/templates/org/settings/navbar.tmpl @@ -4,9 +4,11 @@ {{.i18n.Tr "org.settings.options"}} + {{if not DisableWebhooks}} {{.i18n.Tr "repo.settings.hooks"}} + {{end}} {{.i18n.Tr "repo.labels"}} diff --git a/templates/repo/settings/nav.tmpl b/templates/repo/settings/nav.tmpl index 5cc77e1dc9..4b89ece349 100644 --- a/templates/repo/settings/nav.tmpl +++ b/templates/repo/settings/nav.tmpl @@ -5,7 +5,9 @@
  • {{.i18n.Tr "repo.settings.options"}}
  • {{.i18n.Tr "repo.settings.collaboration"}}
  • {{.i18n.Tr "repo.settings.branches"}}
    • + {{if not DisableWebhooks}}
  • {{.i18n.Tr "repo.settings.hooks"}}
    • + {{end}} {{if or .SignedUser.AllowGitHook .SignedUser.IsAdmin}}
  • {{.i18n.Tr "repo.settings.githooks"}}
    • {{end}} diff --git a/templates/repo/settings/navbar.tmpl b/templates/repo/settings/navbar.tmpl index 1aba5de731..501c3c4630 100644 --- a/templates/repo/settings/navbar.tmpl +++ b/templates/repo/settings/navbar.tmpl @@ -11,9 +11,11 @@ {{.i18n.Tr "repo.settings.branches"}} {{end}} - - {{.i18n.Tr "repo.settings.hooks"}} - + {{if not DisableWebhooks}} + + {{.i18n.Tr "repo.settings.hooks"}} + + {{end}} {{if .SignedUser.CanEditGitHook}} {{.i18n.Tr "repo.settings.githooks"}} -- 2.39.5