Skip to content

Commit

Permalink
add validate webhhook address
Browse files Browse the repository at this point in the history
Signed-off-by: lengrongfu <[email protected]>
  • Loading branch information
lengrongfu committed Aug 2, 2023
1 parent 39ec1e4 commit da93fce
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 9 deletions.
19 changes: 19 additions & 0 deletions src/server/v2.0/handler/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ package handler
import (
"context"
"fmt"
"net/http"
"strings"
"time"

"github.com/go-openapi/runtime/middleware"
"github.com/go-openapi/strfmt"
Expand Down Expand Up @@ -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)
}

Check warning on line 421 in src/server/v2.0/handler/webhook.go

View check run for this annotation

Codecov / codecov/patch

src/server/v2.0/handler/webhook.go#L420-L421

Added lines #L420 - L421 were not covered by tests

if !isNotifyTypeSupported(target.Type) {
return false, errors.New(nil).WithMessage("unsupported target type %s with policy %s", target.Type, policy.Name).WithCode(errors.BadRequestCode)
}
Expand Down Expand Up @@ -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 {
Expand Down
32 changes: 30 additions & 2 deletions src/server/v2.0/handler/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package handler
import (
"fmt"
"io"
"net/http"
"net/http/httptest"
"testing"
"time"

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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{})
}
8 changes: 1 addition & 7 deletions tests/apitests/python/test_webhook_crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit da93fce

Please sign in to comment.