From bc395ac9b2f7444656916404ccf751799ac15d14 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Mon, 7 Aug 2023 18:00:01 -0700 Subject: [PATCH 01/13] support for whitelisting --- internal/api/controller/webhook/common.go | 14 +++++++++-- internal/api/controller/webhook/controller.go | 24 ++++++++++--------- internal/api/controller/webhook/create.go | 6 ++--- internal/api/controller/webhook/update.go | 6 ++--- internal/api/controller/webhook/wire.go | 2 +- internal/services/webhook/http_client.go | 15 ++++++++++-- internal/services/webhook/service.go | 15 ++++++------ internal/services/webhook/trigger.go | 1 + 8 files changed, 54 insertions(+), 29 deletions(-) diff --git a/internal/api/controller/webhook/common.go b/internal/api/controller/webhook/common.go index 87a79273f..6e1b4a9f9 100644 --- a/internal/api/controller/webhook/common.go +++ b/internal/api/controller/webhook/common.go @@ -7,6 +7,7 @@ package webhook import ( "net" "net/url" + "strings" "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" @@ -20,7 +21,7 @@ const ( ) // checkURL validates the url of a webhook. -func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool) error { +func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool, whitelistedInternalUrlPattern []string) error { // check URL if len(rawURL) > webhookMaxURLLength { return check.NewValidationErrorf("The URL of a webhook can be at most %d characters long.", @@ -49,7 +50,7 @@ func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool) error return check.NewValidationError("Loopback IP addresses are not allowed.") } - if !allowPrivateNetwork && ip.IsPrivate() { + if !allowPrivateNetwork && ip.IsPrivate() && !checkWhitelistedUrl(rawURL, whitelistedInternalUrlPattern) { return check.NewValidationError("Private IP addresses are not allowed.") } } @@ -61,6 +62,15 @@ func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool) error return nil } +func checkWhitelistedUrl(URL string, whitelistedInternalUrlPattern []string) bool { + for _, urlPattern := range whitelistedInternalUrlPattern { + if strings.ContainsAny(URL, urlPattern) { + return true + } + } + return false +} + // checkSecret validates the secret of a webhook. func checkSecret(secret string) error { if len(secret) > webhookMaxSecretLength { diff --git a/internal/api/controller/webhook/controller.go b/internal/api/controller/webhook/controller.go index 31162adaa..f876c0ac7 100644 --- a/internal/api/controller/webhook/controller.go +++ b/internal/api/controller/webhook/controller.go @@ -21,8 +21,9 @@ import ( ) type Controller struct { - allowLoopback bool - allowPrivateNetwork bool + allowLoopback bool + allowPrivateNetwork bool + whitelistedInternalUrlPattern []string db *sqlx.DB authorizer authz.Authorizer @@ -35,6 +36,7 @@ type Controller struct { func NewController( allowLoopback bool, allowPrivateNetwork bool, + whitelistedInternalUrlPattern []string, db *sqlx.DB, authorizer authz.Authorizer, webhookStore store.WebhookStore, @@ -43,15 +45,15 @@ func NewController( webhookService *webhook.Service, ) *Controller { return &Controller{ - allowLoopback: allowLoopback, - allowPrivateNetwork: allowPrivateNetwork, - - db: db, - authorizer: authorizer, - webhookStore: webhookStore, - webhookExecutionStore: webhookExecutionStore, - repoStore: repoStore, - webhookService: webhookService, + allowLoopback: allowLoopback, + allowPrivateNetwork: allowPrivateNetwork, + whitelistedInternalUrlPattern: whitelistedInternalUrlPattern, + db: db, + authorizer: authorizer, + webhookStore: webhookStore, + webhookExecutionStore: webhookExecutionStore, + repoStore: repoStore, + webhookService: webhookService, } } diff --git a/internal/api/controller/webhook/create.go b/internal/api/controller/webhook/create.go index e90bcbb4e..aece367e7 100644 --- a/internal/api/controller/webhook/create.go +++ b/internal/api/controller/webhook/create.go @@ -39,7 +39,7 @@ func (c *Controller) Create( } // validate input - err = checkCreateInput(in, c.allowLoopback, c.allowPrivateNetwork) + err = checkCreateInput(in, c.allowLoopback, c.allowPrivateNetwork, c.whitelistedInternalUrlPattern) if err != nil { return nil, err } @@ -73,14 +73,14 @@ func (c *Controller) Create( return hook, nil } -func checkCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwork bool) error { +func checkCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwork bool, whitelistedInternalUrlPattern []string) error { if err := check.DisplayName(in.DisplayName); err != nil { return err } if err := check.Description(in.Description); err != nil { return err } - if err := checkURL(in.URL, allowLoopback, allowPrivateNetwork); err != nil { + if err := checkURL(in.URL, allowLoopback, allowPrivateNetwork, whitelistedInternalUrlPattern); err != nil { return err } if err := checkSecret(in.Secret); err != nil { diff --git a/internal/api/controller/webhook/update.go b/internal/api/controller/webhook/update.go index 0dd55bb94..149cd2444 100644 --- a/internal/api/controller/webhook/update.go +++ b/internal/api/controller/webhook/update.go @@ -43,7 +43,7 @@ func (c *Controller) Update( } // validate input - if err = checkUpdateInput(in, c.allowLoopback, c.allowPrivateNetwork); err != nil { + if err = checkUpdateInput(in, c.allowLoopback, c.allowPrivateNetwork, c.whitelistedInternalUrlPattern); err != nil { return nil, err } @@ -77,7 +77,7 @@ func (c *Controller) Update( return hook, nil } -func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork bool) error { +func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork bool, whitelistedInternalUrlPattern []string) error { if in.DisplayName != nil { if err := check.DisplayName(*in.DisplayName); err != nil { return err @@ -89,7 +89,7 @@ func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork b } } if in.URL != nil { - if err := checkURL(*in.URL, allowLoopback, allowPrivateNetwork); err != nil { + if err := checkURL(*in.URL, allowLoopback, allowPrivateNetwork, whitelistedInternalUrlPattern); err != nil { return err } } diff --git a/internal/api/controller/webhook/wire.go b/internal/api/controller/webhook/wire.go index 95c69ccde..83e314466 100644 --- a/internal/api/controller/webhook/wire.go +++ b/internal/api/controller/webhook/wire.go @@ -21,6 +21,6 @@ var WireSet = wire.NewSet( func ProvideController(config webhook.Config, db *sqlx.DB, authorizer authz.Authorizer, webhookStore store.WebhookStore, webhookExecutionStore store.WebhookExecutionStore, repoStore store.RepoStore, webhookService *webhook.Service) *Controller { - return NewController(config.AllowLoopback, config.AllowPrivateNetwork, + return NewController(config.AllowLoopback, config.AllowPrivateNetwork, config.WhitelistedInternalUrlPattern, db, authorizer, webhookStore, webhookExecutionStore, repoStore, webhookService) } diff --git a/internal/services/webhook/http_client.go b/internal/services/webhook/http_client.go index f78aa5e5a..0bf962c64 100644 --- a/internal/services/webhook/http_client.go +++ b/internal/services/webhook/http_client.go @@ -10,6 +10,7 @@ import ( "fmt" "net" "net/http" + "strings" "time" "github.com/rs/zerolog/log" @@ -20,7 +21,7 @@ var ( errPrivateNetworkNotAllowed = errors.New("private network not allowed") ) -func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerification bool) *http.Client { +func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerification bool, whitelistedInternalUrlPattern []string) *http.Client { // no customizations? use default client if allowLoopback && allowPrivateNetwork && !disableSSLVerification { return http.DefaultClient @@ -76,7 +77,7 @@ func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerif return nil, errLoopbackNotAllowed } - if !allowPrivateNetwork && tcpAddr.IP.IsPrivate() { + if !allowPrivateNetwork && tcpAddr.IP.IsPrivate() && !checkWhitelistedUrl(addr, whitelistedInternalUrlPattern) { return nil, errPrivateNetworkNotAllowed } @@ -89,3 +90,13 @@ func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerif // httpClient is similar to http.DefaultClient, just with custom http.Transport return &http.Client{Transport: tr} } + +func checkWhitelistedUrl(URL string, whitelistedInternalUrlPattern []string) bool { + log.Error().Msgf("url is %s", URL) + for _, urlPattern := range whitelistedInternalUrlPattern { + if strings.ContainsAny(URL, urlPattern) { + return true + } + } + return false +} diff --git a/internal/services/webhook/service.go b/internal/services/webhook/service.go index 6c10c11f1..be4a60367 100644 --- a/internal/services/webhook/service.go +++ b/internal/services/webhook/service.go @@ -33,11 +33,12 @@ type Config struct { HeaderIdentity string `envconfig:"GITNESS_WEBHOOK_HEADER_IDENTITY"` // EventReaderName is the name used to read events from stream. // Note: this should be different for every running instance. - EventReaderName string `envconfig:"GITNESS_WEBHOOK_EVENT_READER_NAME"` - Concurrency int `envconfig:"GITNESS_WEBHOOK_CONCURRENCY" default:"4"` - MaxRetries int `envconfig:"GITNESS_WEBHOOK_MAX_RETRIES" default:"3"` - AllowPrivateNetwork bool `envconfig:"GITNESS_WEBHOOK_ALLOW_PRIVATE_NETWORK" default:"false"` - AllowLoopback bool `envconfig:"GITNESS_WEBHOOK_ALLOW_LOOPBACK" default:"false"` + EventReaderName string `envconfig:"GITNESS_WEBHOOK_EVENT_READER_NAME"` + Concurrency int `envconfig:"GITNESS_WEBHOOK_CONCURRENCY" default:"4"` + MaxRetries int `envconfig:"GITNESS_WEBHOOK_MAX_RETRIES" default:"3"` + AllowPrivateNetwork bool `envconfig:"GITNESS_WEBHOOK_ALLOW_PRIVATE_NETWORK" default:"false"` + AllowLoopback bool `envconfig:"GITNESS_WEBHOOK_ALLOW_LOOPBACK" default:"false"` + WhitelistedInternalUrlPattern []string `envconfig:"GITNESS_WEBHOOK_WHITELISTED_INTERNAL_URL_PATTERNS"` } func (c *Config) Prepare() error { @@ -99,8 +100,8 @@ func NewService(ctx context.Context, config Config, principalStore: principalStore, gitRPCClient: gitRPCClient, - secureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, false), - insecureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, true), + secureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, false, config.WhitelistedInternalUrlPattern), + insecureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, true, config.WhitelistedInternalUrlPattern), config: config, } diff --git a/internal/services/webhook/trigger.go b/internal/services/webhook/trigger.go index e8745d3ca..f29bfefc2 100644 --- a/internal/services/webhook/trigger.go +++ b/internal/services/webhook/trigger.go @@ -357,6 +357,7 @@ func handleWebhookResponse(execution *types.WebhookExecution, resp *http.Respons // store status (handle status later - want to first read body) execution.Response.StatusCode = resp.StatusCode execution.Response.Status = resp.Status + log.Error().Msgf("resp nav %v: %v", execution, resp) // store response headers hBuff := &bytes.Buffer{} From d984848524f23834555af35d88f308c222319a24 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Mon, 7 Aug 2023 18:08:46 -0700 Subject: [PATCH 02/13] support for whitelisting --- internal/services/webhook/service.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/services/webhook/service.go b/internal/services/webhook/service.go index be4a60367..dbb4bc843 100644 --- a/internal/services/webhook/service.go +++ b/internal/services/webhook/service.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "github.com/rs/zerolog/log" "net/http" "time" @@ -105,6 +106,7 @@ func NewService(ctx context.Context, config Config, config: config, } + log.Ctx(ctx).Info().Msgf("Whitelisted internal URL patterns are %v", config.WhitelistedInternalUrlPattern) _, err := gitReaderFactory.Launch(ctx, eventsReaderGroupName, config.EventReaderName, func(r *gitevents.Reader) error { From 7fa167c408fb13a5d7c6bcd36ec0e5c4ed858e8a Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Tue, 15 Aug 2023 00:43:05 -0700 Subject: [PATCH 03/13] support for whitelisting --- internal/services/webhook/trigger.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/services/webhook/trigger.go b/internal/services/webhook/trigger.go index f29bfefc2..e8745d3ca 100644 --- a/internal/services/webhook/trigger.go +++ b/internal/services/webhook/trigger.go @@ -357,7 +357,6 @@ func handleWebhookResponse(execution *types.WebhookExecution, resp *http.Respons // store status (handle status later - want to first read body) execution.Response.StatusCode = resp.StatusCode execution.Response.Status = resp.Status - log.Error().Msgf("resp nav %v: %v", execution, resp) // store response headers hBuff := &bytes.Buffer{} From 257b57ad400a04c6d2448c31324414674407eb5f Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Tue, 15 Aug 2023 01:43:53 -0700 Subject: [PATCH 04/13] support for whitelisting --- internal/api/controller/webhook/controller.go | 13 +++++++++++++ internal/api/controller/webhook/create.go | 4 ++++ internal/api/controller/webhook/update.go | 4 ++++ 3 files changed, 21 insertions(+) diff --git a/internal/api/controller/webhook/controller.go b/internal/api/controller/webhook/controller.go index f876c0ac7..fbbee08e0 100644 --- a/internal/api/controller/webhook/controller.go +++ b/internal/api/controller/webhook/controller.go @@ -7,6 +7,7 @@ package webhook import ( "context" "fmt" + "strings" apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/api/usererror" @@ -74,3 +75,15 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context, return repo, nil } + +func (c *Controller) checkProtectedURLs(session *auth.Session, webhookURL *string) error { + for _, urlPattern := range c.whitelistedInternalUrlPattern { + if strings.Contains(strings.ToLower(*webhookURL), strings.ToLower(urlPattern)) { + if session.Principal.Type != enum.PrincipalTypeService { + return usererror.BadRequest("An internal URL provided") + } + break + } + } + return nil +} diff --git a/internal/api/controller/webhook/create.go b/internal/api/controller/webhook/create.go index aece367e7..505d85352 100644 --- a/internal/api/controller/webhook/create.go +++ b/internal/api/controller/webhook/create.go @@ -38,6 +38,10 @@ func (c *Controller) Create( return nil, err } + err = c.checkProtectedURLs(session, &in.URL) + if err != nil { + return nil, err + } // validate input err = checkCreateInput(in, c.allowLoopback, c.allowPrivateNetwork, c.whitelistedInternalUrlPattern) if err != nil { diff --git a/internal/api/controller/webhook/update.go b/internal/api/controller/webhook/update.go index 149cd2444..9c5082108 100644 --- a/internal/api/controller/webhook/update.go +++ b/internal/api/controller/webhook/update.go @@ -47,6 +47,10 @@ func (c *Controller) Update( return nil, err } + err = c.checkProtectedURLs(session, in.URL) + if err != nil { + return nil, err + } // update webhook struct (only for values that are provided) if in.DisplayName != nil { hook.DisplayName = *in.DisplayName From 56bd32d4c60b08746947cfc975a6288afe14faec Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Tue, 15 Aug 2023 21:48:40 -0700 Subject: [PATCH 05/13] support for whitelisting --- internal/api/controller/webhook/common.go | 19 ++-------- internal/api/controller/webhook/controller.go | 37 +++++-------------- internal/api/controller/webhook/create.go | 16 ++++---- internal/api/controller/webhook/update.go | 8 ++-- internal/api/controller/webhook/wire.go | 2 +- internal/router/api.go | 4 +- internal/services/webhook/http_client.go | 15 +------- internal/services/webhook/service.go | 20 +++++----- internal/services/webhook/trigger.go | 4 +- ..._alter_table_webhook_add_internal_down.sql | 1 + ...20_alter_table_webhook_add_internal_up.sql | 2 + ..._alter_table_webhook_add_internal_down.sql | 1 + ...20_alter_table_webhook_add_internal_up.sql | 2 + internal/store/database/webhook.go | 7 +++- types/webhook.go | 1 + 15 files changed, 57 insertions(+), 82 deletions(-) create mode 100644 internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_down.sql create mode 100644 internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_up.sql create mode 100644 internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_down.sql create mode 100644 internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_up.sql diff --git a/internal/api/controller/webhook/common.go b/internal/api/controller/webhook/common.go index 6e1b4a9f9..fbbe852b6 100644 --- a/internal/api/controller/webhook/common.go +++ b/internal/api/controller/webhook/common.go @@ -5,12 +5,10 @@ package webhook import ( - "net" - "net/url" - "strings" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" + "net" + "net/url" ) const ( @@ -21,7 +19,7 @@ const ( ) // checkURL validates the url of a webhook. -func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool, whitelistedInternalUrlPattern []string) error { +func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool) error { // check URL if len(rawURL) > webhookMaxURLLength { return check.NewValidationErrorf("The URL of a webhook can be at most %d characters long.", @@ -50,7 +48,7 @@ func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool, white return check.NewValidationError("Loopback IP addresses are not allowed.") } - if !allowPrivateNetwork && ip.IsPrivate() && !checkWhitelistedUrl(rawURL, whitelistedInternalUrlPattern) { + if !allowPrivateNetwork && ip.IsPrivate() { return check.NewValidationError("Private IP addresses are not allowed.") } } @@ -62,15 +60,6 @@ func checkURL(rawURL string, allowLoopback bool, allowPrivateNetwork bool, white return nil } -func checkWhitelistedUrl(URL string, whitelistedInternalUrlPattern []string) bool { - for _, urlPattern := range whitelistedInternalUrlPattern { - if strings.ContainsAny(URL, urlPattern) { - return true - } - } - return false -} - // checkSecret validates the secret of a webhook. func checkSecret(secret string) error { if len(secret) > webhookMaxSecretLength { diff --git a/internal/api/controller/webhook/controller.go b/internal/api/controller/webhook/controller.go index fbbee08e0..2c392b559 100644 --- a/internal/api/controller/webhook/controller.go +++ b/internal/api/controller/webhook/controller.go @@ -7,8 +7,6 @@ package webhook import ( "context" "fmt" - "strings" - apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/api/usererror" "github.com/harness/gitness/internal/auth" @@ -22,9 +20,8 @@ import ( ) type Controller struct { - allowLoopback bool - allowPrivateNetwork bool - whitelistedInternalUrlPattern []string + allowLoopback bool + allowPrivateNetwork bool db *sqlx.DB authorizer authz.Authorizer @@ -37,7 +34,6 @@ type Controller struct { func NewController( allowLoopback bool, allowPrivateNetwork bool, - whitelistedInternalUrlPattern []string, db *sqlx.DB, authorizer authz.Authorizer, webhookStore store.WebhookStore, @@ -46,15 +42,14 @@ func NewController( webhookService *webhook.Service, ) *Controller { return &Controller{ - allowLoopback: allowLoopback, - allowPrivateNetwork: allowPrivateNetwork, - whitelistedInternalUrlPattern: whitelistedInternalUrlPattern, - db: db, - authorizer: authorizer, - webhookStore: webhookStore, - webhookExecutionStore: webhookExecutionStore, - repoStore: repoStore, - webhookService: webhookService, + allowLoopback: allowLoopback, + allowPrivateNetwork: allowPrivateNetwork, + db: db, + authorizer: authorizer, + webhookStore: webhookStore, + webhookExecutionStore: webhookExecutionStore, + repoStore: repoStore, + webhookService: webhookService, } } @@ -75,15 +70,3 @@ func (c *Controller) getRepoCheckAccess(ctx context.Context, return repo, nil } - -func (c *Controller) checkProtectedURLs(session *auth.Session, webhookURL *string) error { - for _, urlPattern := range c.whitelistedInternalUrlPattern { - if strings.Contains(strings.ToLower(*webhookURL), strings.ToLower(urlPattern)) { - if session.Principal.Type != enum.PrincipalTypeService { - return usererror.BadRequest("An internal URL provided") - } - break - } - } - return nil -} diff --git a/internal/api/controller/webhook/create.go b/internal/api/controller/webhook/create.go index 505d85352..036e0fa31 100644 --- a/internal/api/controller/webhook/create.go +++ b/internal/api/controller/webhook/create.go @@ -22,6 +22,7 @@ type CreateInput struct { Enabled bool `json:"enabled"` Insecure bool `json:"insecure"` Triggers []enum.WebhookTrigger `json:"triggers"` + Internal bool `json:"internal"` } // Create creates a new webhook. @@ -38,12 +39,8 @@ func (c *Controller) Create( return nil, err } - err = c.checkProtectedURLs(session, &in.URL) - if err != nil { - return nil, err - } // validate input - err = checkCreateInput(in, c.allowLoopback, c.allowPrivateNetwork, c.whitelistedInternalUrlPattern) + err = checkCreateInput(in, c.allowLoopback, c.allowPrivateNetwork || in.Internal) if err != nil { return nil, err } @@ -57,6 +54,7 @@ func (c *Controller) Create( Updated: now, ParentID: repo.ID, ParentType: enum.WebhookParentRepo, + Internal: in.Internal, // user input DisplayName: in.DisplayName, @@ -77,15 +75,17 @@ func (c *Controller) Create( return hook, nil } -func checkCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwork bool, whitelistedInternalUrlPattern []string) error { +func checkCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwork bool) error { if err := check.DisplayName(in.DisplayName); err != nil { return err } if err := check.Description(in.Description); err != nil { return err } - if err := checkURL(in.URL, allowLoopback, allowPrivateNetwork, whitelistedInternalUrlPattern); err != nil { - return err + if !in.Internal { + if err := checkURL(in.URL, allowLoopback, allowPrivateNetwork); err != nil { + return err + } } if err := checkSecret(in.Secret); err != nil { return err diff --git a/internal/api/controller/webhook/update.go b/internal/api/controller/webhook/update.go index 9c5082108..7962a124d 100644 --- a/internal/api/controller/webhook/update.go +++ b/internal/api/controller/webhook/update.go @@ -21,6 +21,7 @@ type UpdateInput struct { Enabled *bool `json:"enabled"` Insecure *bool `json:"insecure"` Triggers []enum.WebhookTrigger `json:"triggers"` + Internal *bool `json:"internal"` } // Update updates an existing webhook. @@ -43,11 +44,10 @@ func (c *Controller) Update( } // validate input - if err = checkUpdateInput(in, c.allowLoopback, c.allowPrivateNetwork, c.whitelistedInternalUrlPattern); err != nil { + if err = checkUpdateInput(in, c.allowLoopback, c.allowPrivateNetwork || *in.Internal); err != nil { return nil, err } - err = c.checkProtectedURLs(session, in.URL) if err != nil { return nil, err } @@ -81,7 +81,7 @@ func (c *Controller) Update( return hook, nil } -func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork bool, whitelistedInternalUrlPattern []string) error { +func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork bool) error { if in.DisplayName != nil { if err := check.DisplayName(*in.DisplayName); err != nil { return err @@ -93,7 +93,7 @@ func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork b } } if in.URL != nil { - if err := checkURL(*in.URL, allowLoopback, allowPrivateNetwork, whitelistedInternalUrlPattern); err != nil { + if err := checkURL(*in.URL, allowLoopback, allowPrivateNetwork); err != nil { return err } } diff --git a/internal/api/controller/webhook/wire.go b/internal/api/controller/webhook/wire.go index 83e314466..95c69ccde 100644 --- a/internal/api/controller/webhook/wire.go +++ b/internal/api/controller/webhook/wire.go @@ -21,6 +21,6 @@ var WireSet = wire.NewSet( func ProvideController(config webhook.Config, db *sqlx.DB, authorizer authz.Authorizer, webhookStore store.WebhookStore, webhookExecutionStore store.WebhookExecutionStore, repoStore store.RepoStore, webhookService *webhook.Service) *Controller { - return NewController(config.AllowLoopback, config.AllowPrivateNetwork, config.WhitelistedInternalUrlPattern, + return NewController(config.AllowLoopback, config.AllowPrivateNetwork, db, authorizer, webhookStore, webhookExecutionStore, repoStore, webhookService) } diff --git a/internal/router/api.go b/internal/router/api.go index 35dc4238d..40e3c1823 100644 --- a/internal/router/api.go +++ b/internal/router/api.go @@ -259,7 +259,7 @@ func setupRepos(r chi.Router, SetupPullReq(r, pullreqCtrl) - SetupWebhook(r, webhookCtrl) + setupWebhook(r, webhookCtrl) SetupChecks(r, checkCtrl) }) @@ -315,7 +315,7 @@ func SetupPullReq(r chi.Router, pullreqCtrl *pullreq.Controller) { }) } -func SetupWebhook(r chi.Router, webhookCtrl *webhook.Controller) { +func setupWebhook(r chi.Router, webhookCtrl *webhook.Controller) { r.Route("/webhooks", func(r chi.Router) { r.Post("/", handlerwebhook.HandleCreate(webhookCtrl)) r.Get("/", handlerwebhook.HandleList(webhookCtrl)) diff --git a/internal/services/webhook/http_client.go b/internal/services/webhook/http_client.go index 0bf962c64..f78aa5e5a 100644 --- a/internal/services/webhook/http_client.go +++ b/internal/services/webhook/http_client.go @@ -10,7 +10,6 @@ import ( "fmt" "net" "net/http" - "strings" "time" "github.com/rs/zerolog/log" @@ -21,7 +20,7 @@ var ( errPrivateNetworkNotAllowed = errors.New("private network not allowed") ) -func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerification bool, whitelistedInternalUrlPattern []string) *http.Client { +func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerification bool) *http.Client { // no customizations? use default client if allowLoopback && allowPrivateNetwork && !disableSSLVerification { return http.DefaultClient @@ -77,7 +76,7 @@ func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerif return nil, errLoopbackNotAllowed } - if !allowPrivateNetwork && tcpAddr.IP.IsPrivate() && !checkWhitelistedUrl(addr, whitelistedInternalUrlPattern) { + if !allowPrivateNetwork && tcpAddr.IP.IsPrivate() { return nil, errPrivateNetworkNotAllowed } @@ -90,13 +89,3 @@ func newHTTPClient(allowLoopback bool, allowPrivateNetwork bool, disableSSLVerif // httpClient is similar to http.DefaultClient, just with custom http.Transport return &http.Client{Transport: tr} } - -func checkWhitelistedUrl(URL string, whitelistedInternalUrlPattern []string) bool { - log.Error().Msgf("url is %s", URL) - for _, urlPattern := range whitelistedInternalUrlPattern { - if strings.ContainsAny(URL, urlPattern) { - return true - } - } - return false -} diff --git a/internal/services/webhook/service.go b/internal/services/webhook/service.go index dbb4bc843..499f10538 100644 --- a/internal/services/webhook/service.go +++ b/internal/services/webhook/service.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "github.com/rs/zerolog/log" "net/http" "time" @@ -34,12 +33,11 @@ type Config struct { HeaderIdentity string `envconfig:"GITNESS_WEBHOOK_HEADER_IDENTITY"` // EventReaderName is the name used to read events from stream. // Note: this should be different for every running instance. - EventReaderName string `envconfig:"GITNESS_WEBHOOK_EVENT_READER_NAME"` - Concurrency int `envconfig:"GITNESS_WEBHOOK_CONCURRENCY" default:"4"` - MaxRetries int `envconfig:"GITNESS_WEBHOOK_MAX_RETRIES" default:"3"` - AllowPrivateNetwork bool `envconfig:"GITNESS_WEBHOOK_ALLOW_PRIVATE_NETWORK" default:"false"` - AllowLoopback bool `envconfig:"GITNESS_WEBHOOK_ALLOW_LOOPBACK" default:"false"` - WhitelistedInternalUrlPattern []string `envconfig:"GITNESS_WEBHOOK_WHITELISTED_INTERNAL_URL_PATTERNS"` + EventReaderName string `envconfig:"GITNESS_WEBHOOK_EVENT_READER_NAME"` + Concurrency int `envconfig:"GITNESS_WEBHOOK_CONCURRENCY" default:"4"` + MaxRetries int `envconfig:"GITNESS_WEBHOOK_MAX_RETRIES" default:"3"` + AllowPrivateNetwork bool `envconfig:"GITNESS_WEBHOOK_ALLOW_PRIVATE_NETWORK" default:"false"` + AllowLoopback bool `envconfig:"GITNESS_WEBHOOK_ALLOW_LOOPBACK" default:"false"` } func (c *Config) Prepare() error { @@ -80,6 +78,8 @@ type Service struct { secureHTTPClient *http.Client insecureHTTPClient *http.Client + secureHTTPClientInternal *http.Client + config Config } @@ -101,12 +101,12 @@ func NewService(ctx context.Context, config Config, principalStore: principalStore, gitRPCClient: gitRPCClient, - secureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, false, config.WhitelistedInternalUrlPattern), - insecureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, true, config.WhitelistedInternalUrlPattern), + secureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, false), + insecureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, true), + secureHTTPClientInternal: newHTTPClient(config.AllowLoopback, true, false), config: config, } - log.Ctx(ctx).Info().Msgf("Whitelisted internal URL patterns are %v", config.WhitelistedInternalUrlPattern) _, err := gitReaderFactory.Launch(ctx, eventsReaderGroupName, config.EventReaderName, func(r *gitevents.Reader) error { diff --git a/internal/services/webhook/trigger.go b/internal/services/webhook/trigger.go index e8745d3ca..48ff998c5 100644 --- a/internal/services/webhook/trigger.go +++ b/internal/services/webhook/trigger.go @@ -219,7 +219,9 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr // Execute HTTP Request (insecure if requested) var resp *http.Response - if webhook.Insecure { + if webhook.Internal { + resp, err = s.secureHTTPClientInternal.Do(req) + } else if webhook.Insecure { resp, err = s.insecureHTTPClient.Do(req) } else { resp, err = s.secureHTTPClient.Do(req) diff --git a/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_down.sql b/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_down.sql new file mode 100644 index 000000000..68d596a51 --- /dev/null +++ b/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_down.sql @@ -0,0 +1 @@ +ALTER TABLE webhooks DROP COLUMN webhook_internal; diff --git a/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_up.sql b/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_up.sql new file mode 100644 index 000000000..f11891414 --- /dev/null +++ b/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_up.sql @@ -0,0 +1,2 @@ +ALTER TABLE webhooks + ADD COLUMN webhook_internal BOOLEAN NOT NULL DEFAULT false; diff --git a/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_down.sql b/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_down.sql new file mode 100644 index 000000000..68d596a51 --- /dev/null +++ b/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_down.sql @@ -0,0 +1 @@ +ALTER TABLE webhooks DROP COLUMN webhook_internal; diff --git a/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_up.sql b/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_up.sql new file mode 100644 index 000000000..e0d32cb7c --- /dev/null +++ b/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_up.sql @@ -0,0 +1,2 @@ +ALTER TABLE webhooks + ADD COLUMN webhook_internal BOOLEAN NOT NULL DEFAULT false; \ No newline at end of file diff --git a/internal/store/database/webhook.go b/internal/store/database/webhook.go index 05fd68929..f5754b1a4 100644 --- a/internal/store/database/webhook.go +++ b/internal/store/database/webhook.go @@ -45,6 +45,7 @@ type webhook struct { CreatedBy int64 `db:"webhook_created_by"` Created int64 `db:"webhook_created"` Updated int64 `db:"webhook_updated"` + Internal bool `db:"webhook_internal"` DisplayName string `db:"webhook_display_name"` Description string `db:"webhook_description"` @@ -72,7 +73,8 @@ const ( ,webhook_enabled ,webhook_insecure ,webhook_triggers - ,webhook_latest_execution_result` + ,webhook_latest_execution_result + ,webhook_internal` webhookSelectBase = ` SELECT` + webhookColumns + ` @@ -116,6 +118,7 @@ func (s *WebhookStore) Create(ctx context.Context, hook *types.Webhook) error { ,webhook_insecure ,webhook_triggers ,webhook_latest_execution_result + ,webhook_internal ) values ( :webhook_repo_id ,:webhook_space_id @@ -130,6 +133,7 @@ func (s *WebhookStore) Create(ctx context.Context, hook *types.Webhook) error { ,:webhook_insecure ,:webhook_triggers ,:webhook_latest_execution_result + ,:webhook_internal ) RETURNING webhook_id` db := dbtx.GetAccessor(ctx, s.db) @@ -166,6 +170,7 @@ func (s *WebhookStore) Update(ctx context.Context, hook *types.Webhook) error { ,webhook_insecure = :webhook_insecure ,webhook_triggers = :webhook_triggers ,webhook_latest_execution_result = :webhook_latest_execution_result + ,webhook_internal = :webhook_internal WHERE webhook_id = :webhook_id and webhook_version = :webhook_version - 1` db := dbtx.GetAccessor(ctx, s.db) diff --git a/types/webhook.go b/types/webhook.go index 5b8ec9f99..29d7d7ef3 100644 --- a/types/webhook.go +++ b/types/webhook.go @@ -19,6 +19,7 @@ type Webhook struct { CreatedBy int64 `json:"created_by"` Created int64 `json:"created"` Updated int64 `json:"updated"` + Internal bool `json:"internal"` DisplayName string `json:"display_name"` Description string `json:"description"` From 435335ba1983390a7a2a981091712b8f738dafba Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Tue, 15 Aug 2023 21:49:53 -0700 Subject: [PATCH 06/13] support for whitelisting --- internal/api/controller/webhook/common.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/api/controller/webhook/common.go b/internal/api/controller/webhook/common.go index fbbe852b6..87a79273f 100644 --- a/internal/api/controller/webhook/common.go +++ b/internal/api/controller/webhook/common.go @@ -5,10 +5,11 @@ package webhook import ( - "github.com/harness/gitness/types/check" - "github.com/harness/gitness/types/enum" "net" "net/url" + + "github.com/harness/gitness/types/check" + "github.com/harness/gitness/types/enum" ) const ( From 18212fea3a5331015515bd7bd83d22c83650bb76 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Aug 2023 12:54:13 -0700 Subject: [PATCH 07/13] fix --- ...al_down.sql => 0021_alter_table_webhook_add_internal_down.sql} | 0 ...ternal_up.sql => 0021_alter_table_webhook_add_internal_up.sql} | 0 ...al_down.sql => 0021_alter_table_webhook_add_internal_down.sql} | 0 ...ternal_up.sql => 0021_alter_table_webhook_add_internal_up.sql} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename internal/store/database/migrate/postgres/{0020_alter_table_webhook_add_internal_down.sql => 0021_alter_table_webhook_add_internal_down.sql} (100%) rename internal/store/database/migrate/postgres/{0020_alter_table_webhook_add_internal_up.sql => 0021_alter_table_webhook_add_internal_up.sql} (100%) rename internal/store/database/migrate/sqlite/{0020_alter_table_webhook_add_internal_down.sql => 0021_alter_table_webhook_add_internal_down.sql} (100%) rename internal/store/database/migrate/sqlite/{0020_alter_table_webhook_add_internal_up.sql => 0021_alter_table_webhook_add_internal_up.sql} (100%) diff --git a/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_down.sql b/internal/store/database/migrate/postgres/0021_alter_table_webhook_add_internal_down.sql similarity index 100% rename from internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_down.sql rename to internal/store/database/migrate/postgres/0021_alter_table_webhook_add_internal_down.sql diff --git a/internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_up.sql b/internal/store/database/migrate/postgres/0021_alter_table_webhook_add_internal_up.sql similarity index 100% rename from internal/store/database/migrate/postgres/0020_alter_table_webhook_add_internal_up.sql rename to internal/store/database/migrate/postgres/0021_alter_table_webhook_add_internal_up.sql diff --git a/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_down.sql b/internal/store/database/migrate/sqlite/0021_alter_table_webhook_add_internal_down.sql similarity index 100% rename from internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_down.sql rename to internal/store/database/migrate/sqlite/0021_alter_table_webhook_add_internal_down.sql diff --git a/internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_up.sql b/internal/store/database/migrate/sqlite/0021_alter_table_webhook_add_internal_up.sql similarity index 100% rename from internal/store/database/migrate/sqlite/0020_alter_table_webhook_add_internal_up.sql rename to internal/store/database/migrate/sqlite/0021_alter_table_webhook_add_internal_up.sql From debb7546af4b1ee88e92bfb97d6b633ade7f8148 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Aug 2023 13:51:58 -0700 Subject: [PATCH 08/13] fix --- internal/api/controller/webhook/create.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/api/controller/webhook/create.go b/internal/api/controller/webhook/create.go index 036e0fa31..4f1476191 100644 --- a/internal/api/controller/webhook/create.go +++ b/internal/api/controller/webhook/create.go @@ -22,7 +22,6 @@ type CreateInput struct { Enabled bool `json:"enabled"` Insecure bool `json:"insecure"` Triggers []enum.WebhookTrigger `json:"triggers"` - Internal bool `json:"internal"` } // Create creates a new webhook. @@ -31,6 +30,7 @@ func (c *Controller) Create( session *auth.Session, repoRef string, in *CreateInput, + internal bool, ) (*types.Webhook, error) { now := time.Now().UnixMilli() @@ -40,7 +40,7 @@ func (c *Controller) Create( } // validate input - err = checkCreateInput(in, c.allowLoopback, c.allowPrivateNetwork || in.Internal) + err = checkCreateInput(in, c.allowLoopback, c.allowPrivateNetwork || internal) if err != nil { return nil, err } @@ -54,7 +54,7 @@ func (c *Controller) Create( Updated: now, ParentID: repo.ID, ParentType: enum.WebhookParentRepo, - Internal: in.Internal, + Internal: internal, // user input DisplayName: in.DisplayName, @@ -82,10 +82,8 @@ func checkCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwork b if err := check.Description(in.Description); err != nil { return err } - if !in.Internal { - if err := checkURL(in.URL, allowLoopback, allowPrivateNetwork); err != nil { - return err - } + if err := checkURL(in.URL, allowLoopback, allowPrivateNetwork); err != nil { + return err } if err := checkSecret(in.Secret); err != nil { return err From d560dcb790cd80334676c4cc39eb4d51a423ce9b Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Aug 2023 13:54:02 -0700 Subject: [PATCH 09/13] fix --- internal/api/handler/webhook/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/handler/webhook/create.go b/internal/api/handler/webhook/create.go index e72178a79..9d05b52cb 100644 --- a/internal/api/handler/webhook/create.go +++ b/internal/api/handler/webhook/create.go @@ -32,7 +32,7 @@ func HandleCreate(webhookCtrl *webhook.Controller) http.HandlerFunc { return } - hook, err := webhookCtrl.Create(ctx, session, repoRef, in) + hook, err := webhookCtrl.Create(ctx, session, repoRef, in, false) if err != nil { render.TranslatedUserError(w, err) return From b6917d9efd8eddfe01e50a338d7cfddf86ea8f43 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Aug 2023 14:09:04 -0700 Subject: [PATCH 10/13] fix --- internal/api/controller/webhook/update.go | 3 +-- internal/services/webhook/trigger.go | 8 +++++--- types/webhook.go | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/api/controller/webhook/update.go b/internal/api/controller/webhook/update.go index 7962a124d..b1d181b79 100644 --- a/internal/api/controller/webhook/update.go +++ b/internal/api/controller/webhook/update.go @@ -21,7 +21,6 @@ type UpdateInput struct { Enabled *bool `json:"enabled"` Insecure *bool `json:"insecure"` Triggers []enum.WebhookTrigger `json:"triggers"` - Internal *bool `json:"internal"` } // Update updates an existing webhook. @@ -44,7 +43,7 @@ func (c *Controller) Update( } // validate input - if err = checkUpdateInput(in, c.allowLoopback, c.allowPrivateNetwork || *in.Internal); err != nil { + if err = checkUpdateInput(in, c.allowLoopback, c.allowPrivateNetwork); err != nil { return nil, err } diff --git a/internal/services/webhook/trigger.go b/internal/services/webhook/trigger.go index 48ff998c5..6e0d078d6 100644 --- a/internal/services/webhook/trigger.go +++ b/internal/services/webhook/trigger.go @@ -221,10 +221,12 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr var resp *http.Response if webhook.Internal { resp, err = s.secureHTTPClientInternal.Do(req) - } else if webhook.Insecure { - resp, err = s.insecureHTTPClient.Do(req) } else { - resp, err = s.secureHTTPClient.Do(req) + if webhook.Insecure { + resp, err = s.insecureHTTPClient.Do(req) + } else { + resp, err = s.secureHTTPClient.Do(req) + } } // always close the body! diff --git a/types/webhook.go b/types/webhook.go index 29d7d7ef3..fca116d07 100644 --- a/types/webhook.go +++ b/types/webhook.go @@ -19,7 +19,7 @@ type Webhook struct { CreatedBy int64 `json:"created_by"` Created int64 `json:"created"` Updated int64 `json:"updated"` - Internal bool `json:"internal"` + Internal bool `json:"-"` DisplayName string `json:"display_name"` Description string `json:"description"` From bc25ad33dc1d5e1293a14c13ec4a05e36c8c2625 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Aug 2023 14:11:06 -0700 Subject: [PATCH 11/13] fix --- internal/api/controller/webhook/update.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/api/controller/webhook/update.go b/internal/api/controller/webhook/update.go index b1d181b79..0dd55bb94 100644 --- a/internal/api/controller/webhook/update.go +++ b/internal/api/controller/webhook/update.go @@ -47,9 +47,6 @@ func (c *Controller) Update( return nil, err } - if err != nil { - return nil, err - } // update webhook struct (only for values that are provided) if in.DisplayName != nil { hook.DisplayName = *in.DisplayName From 7dafe640523018c2de76a143187b94ecbd266f21 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Aug 2023 15:48:48 -0700 Subject: [PATCH 12/13] fix --- internal/services/webhook/trigger.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/services/webhook/trigger.go b/internal/services/webhook/trigger.go index 6e0d078d6..2d1427b56 100644 --- a/internal/services/webhook/trigger.go +++ b/internal/services/webhook/trigger.go @@ -219,14 +219,15 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr // Execute HTTP Request (insecure if requested) var resp *http.Response - if webhook.Internal { + switch { + case webhook.Internal && webhook.Insecure: + err = errors.New("insecure internal webhook not supported") + case webhook.Internal && !webhook.Insecure: resp, err = s.secureHTTPClientInternal.Do(req) - } else { - if webhook.Insecure { - resp, err = s.insecureHTTPClient.Do(req) - } else { - resp, err = s.secureHTTPClient.Do(req) - } + case webhook.Insecure: + resp, err = s.insecureHTTPClient.Do(req) + default: + resp, err = s.secureHTTPClient.Do(req) } // always close the body! From aec30b4e1e1560e2463a69656b8efdd2b3722dfe Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Wed, 16 Aug 2023 15:55:38 -0700 Subject: [PATCH 13/13] fix --- internal/services/webhook/service.go | 11 +++++++---- internal/services/webhook/trigger.go | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/internal/services/webhook/service.go b/internal/services/webhook/service.go index 499f10538..52cd0b7dc 100644 --- a/internal/services/webhook/service.go +++ b/internal/services/webhook/service.go @@ -78,7 +78,8 @@ type Service struct { secureHTTPClient *http.Client insecureHTTPClient *http.Client - secureHTTPClientInternal *http.Client + secureHTTPClientInternal *http.Client + insecureHTTPClientInternal *http.Client config Config } @@ -101,9 +102,11 @@ func NewService(ctx context.Context, config Config, principalStore: principalStore, gitRPCClient: gitRPCClient, - secureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, false), - insecureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, true), - secureHTTPClientInternal: newHTTPClient(config.AllowLoopback, true, false), + secureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, false), + insecureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, true), + + secureHTTPClientInternal: newHTTPClient(config.AllowLoopback, true, false), + insecureHTTPClientInternal: newHTTPClient(config.AllowLoopback, true, true), config: config, } diff --git a/internal/services/webhook/trigger.go b/internal/services/webhook/trigger.go index 2d1427b56..549a2090e 100644 --- a/internal/services/webhook/trigger.go +++ b/internal/services/webhook/trigger.go @@ -221,8 +221,8 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr var resp *http.Response switch { case webhook.Internal && webhook.Insecure: - err = errors.New("insecure internal webhook not supported") - case webhook.Internal && !webhook.Insecure: + resp, err = s.insecureHTTPClientInternal.Do(req) + case webhook.Internal: resp, err = s.secureHTTPClientInternal.Do(req) case webhook.Insecure: resp, err = s.insecureHTTPClient.Do(req)