diff --git a/src/server/v2.0/handler/webhook.go b/src/server/v2.0/handler/webhook.go index 941343b0d843..b429e77d1de0 100644 --- a/src/server/v2.0/handler/webhook.go +++ b/src/server/v2.0/handler/webhook.go @@ -17,7 +17,9 @@ package handler import ( "context" "fmt" + "net/http" "strings" + "time" "github.com/go-openapi/runtime/middleware" "github.com/go-openapi/strfmt" @@ -414,6 +416,10 @@ func (n *webhookAPI) validateTargets(policy *policy_model.Policy) (bool, error) // Prevent SSRF security issue #3755 target.Address = url.Scheme + "://" + url.Host + url.Path + if err := validateAddress(target.Address); err != nil { + return false, errors.New(err).WithCode(errors.BadRequestCode) + } + if !isNotifyTypeSupported(target.Type) { return false, errors.New(nil).WithMessage("unsupported target type %s with policy %s", target.Type, policy.Name).WithCode(errors.BadRequestCode) } @@ -475,6 +481,19 @@ func (n *webhookAPI) constructPolicyWithTriggerTime(ctx context.Context, policie return res, nil } +// validateAddress validate the address is connectable +func validateAddress(address string) error { + request, _ := http.NewRequest("GET", address, nil) + client := http.Client{ + Timeout: time.Second * 3, + } + resp, err := client.Do(request) + if err != nil { + return errors.New(err).WithCode(errors.BadRequestCode) + } + return resp.Body.Close() +} + func isEventTypeSupported(eventType string) bool { for _, t := range notification.GetSupportedEventTypes() { if t.String() == eventType { diff --git a/src/server/v2.0/handler/webhook_test.go b/src/server/v2.0/handler/webhook_test.go index 026e9ac7955b..52ceabe456fd 100644 --- a/src/server/v2.0/handler/webhook_test.go +++ b/src/server/v2.0/handler/webhook_test.go @@ -17,6 +17,8 @@ package handler import ( "fmt" "io" + "net/http" + "net/http/httptest" "testing" "time" @@ -92,7 +94,12 @@ func (suite *WebhookTestSuite) TestCreateWebhookPolicyOfProject() { { // valid policy should got 200 - resp, err := suite.PostJSON(url, &models.WebhookPolicy{EventTypes: []string{"PUSH_ARTIFACT"}, Targets: []*models.WebhookTargetObject{{Type: "http", Address: "http://127.0.0.1"}}}) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "Hello, World!") + }) + server := httptest.NewServer(handler) + defer server.Close() + resp, err := suite.PostJSON(url, &models.WebhookPolicy{EventTypes: []string{"PUSH_ARTIFACT"}, Targets: []*models.WebhookTargetObject{{Type: "http", Address: server.URL}}}) suite.NoError(err) suite.Equal(201, resp.StatusCode) } @@ -121,7 +128,12 @@ func (suite *WebhookTestSuite) TestUpdateWebhookPolicyOfProject() { { // valid policy should got 200 - resp, err := suite.PutJSON(url, &models.WebhookPolicy{EventTypes: []string{"PUSH_ARTIFACT"}, Targets: []*models.WebhookTargetObject{{Type: "http", Address: "http://127.0.0.1"}}}) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "Hello, World!") + }) + server := httptest.NewServer(handler) + defer server.Close() + resp, err := suite.PutJSON(url, &models.WebhookPolicy{EventTypes: []string{"PUSH_ARTIFACT"}, Targets: []*models.WebhookTargetObject{{Type: "http", Address: server.URL}}}) suite.NoError(err) suite.Equal(200, resp.StatusCode) } @@ -232,6 +244,22 @@ func (suite *WebhookTestSuite) TestGetSupportedEventTypes() { suite.Len(body.NotifyType, len(notification.GetSupportedNotifyTypes())) } +func (suite *WebhookTestSuite) Test_validateAddress() { + err := validateAddress("http://123:8080") + suite.Error(err) + + err = validateAddress("https://1.2.3.4") + suite.Error(err) + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "Hello, World!") + }) + server := httptest.NewServer(handler) + defer server.Close() + err = validateAddress(server.URL) + suite.NoError(err) +} + func TestWebhookTestSuite(t *testing.T) { suite.Run(t, &WebhookTestSuite{}) } diff --git a/tests/apitests/python/test_webhook_crud.py b/tests/apitests/python/test_webhook_crud.py index 8fbacc3cbcd1..f87e9c1ba085 100644 --- a/tests/apitests/python/test_webhook_crud.py +++ b/tests/apitests/python/test_webhook_crud.py @@ -58,14 +58,8 @@ def testDelRepo(self): type = "slack", auth_header = "aaa" ) - target_2 = v2_swagger_client.WebhookTargetObject( - address = "https://202.10.12.13", - skip_cert_verify = False, - type = "http", - auth_header = "aaa" - ) #This need to be removed once issue #13378 fixed. - policy_id, policy_name = self.webhook.create_webhook(self.project_id, [target_1, target_2], **self.USER_CLIENT) + policy_id, policy_name = self.webhook.create_webhook(self.project_id, [target_1], **self.USER_CLIENT) target_1 = v2_swagger_client.WebhookTargetObject( address = "https://hooks.slack.com/services/new", skip_cert_verify = True,