Merge pull request #1899 from l3montree-dev/fix/webhook

Fixes webhook integration and improves

Author: timbastin

CLAUDE.md

@@ -303,8 +303,9 @@ func (w *WebhookController) HandleEvent(ctx context.Context, event any) error {
 				//send sbom
 				if err := client.SendSBOM(ctx, *event.SBOM, event.Org, event.Project, event.Asset, event.AssetVersion, event.Artifact); err != nil {
 					slog.Error("failed to send SBOM to webhook", "webhookID", webhook.ID, "err", err)
+				} else {
+					slog.Info("webhook sent", "eventType", "sbom", "webhookID", webhook.ID, "org", event.Org.Name)
 				}
-				slog.Info("SBOM sent to webhook", "webhookID", webhook.ID)
 			}
 		}
 	case shared.FirstPartyVulnsDetectedEvent:
@@ -323,8 +324,9 @@ func (w *WebhookController) HandleEvent(ctx context.Context, event any) error {
 				//send vulnerability
 				if err := client.SendFirstPartyVulnerabilities(ctx, vulns, event.Org, event.Project, event.Asset, event.AssetVersion); err != nil {
 					slog.Error("failed to send vulnerability to webhook", "webhookID", webhook.ID, "err", err)
+				} else {
+					slog.Info("webhook sent", "eventType", "firstPartyVulnerabilities", "webhookID", webhook.ID, "org", event.Org.Name)
 				}
-				slog.Info("Vulnerability sent to webhook", "webhookID", webhook.ID)
 			}
 		}
 
@@ -344,8 +346,9 @@ func (w *WebhookController) HandleEvent(ctx context.Context, event any) error {
 				//send vulnerability
 				if err := client.SendDependencyVulnerabilities(ctx, vulns, event.Org, event.Project, event.Asset, event.AssetVersion, event.Artifact); err != nil {
 					slog.Error("failed to send vulnerability to webhook", "webhookID", webhook.ID, "err", err)
+				} else {
+					slog.Info("webhook sent", "eventType", "dependencyVulnerabilities", "webhookID", webhook.ID, "org", event.Org.Name)
 				}
-				slog.Info("Vulnerability sent to webhook", "webhookID", webhook.ID)
 			}
 		}
 	}

controllers/webhook_controller.go

@@ -16,6 +16,7 @@
 package integrations
 
 import (
+	"github.com/l3montree-dev/devguard/controllers"
 	"github.com/l3montree-dev/devguard/integrations/githubint"
 	"github.com/l3montree-dev/devguard/integrations/gitlabint"
 	"github.com/l3montree-dev/devguard/integrations/jiraint"
@@ -38,8 +39,8 @@ var Module = fx.Options(
 
 	// Aggregated Third Party Integration
 	fx.Provide(fx.Annotate(
-		func(externalUserRepository shared.ExternalUserRepository, gitlabIntegration *gitlabint.GitlabIntegration, githubIntegration *githubint.GithubIntegration, jiraIntegration *jiraint.JiraIntegration) shared.IntegrationAggregate {
-			return NewThirdPartyIntegrations(externalUserRepository, githubIntegration, jiraIntegration, gitlabIntegration)
+		func(externalUserRepository shared.ExternalUserRepository, gitlabIntegration *gitlabint.GitlabIntegration, githubIntegration *githubint.GithubIntegration, jiraIntegration *jiraint.JiraIntegration, webhookIntegration *controllers.WebhookController) shared.IntegrationAggregate {
+			return NewThirdPartyIntegrations(externalUserRepository, githubIntegration, jiraIntegration, gitlabIntegration, webhookIntegration)
 		},
 		fx.As(new(shared.IntegrationAggregate)),
 	)),

integrations/providers.go

@@ -51,16 +51,18 @@ const (
 )
 
 type webhookClient struct {
-	URL        string
-	Secret     *string
-	httpClient *http.Client
+	URL         string
+	Secret      *string
+	httpClient  *http.Client
+	retryDelays []time.Duration
 }
 
 func NewWebhookService(url string, secret *string) *webhookClient {
 	return &webhookClient{
-		URL:        url,
-		Secret:     secret,
-		httpClient: &http.Client{Transport: utils.EgressTransport},
+		URL:         url,
+		Secret:      secret,
+		httpClient:  &http.Client{Transport: utils.EgressTransport},
+		retryDelays: []time.Duration{1 * time.Second, 5 * time.Second, 10 * time.Second},
 	}
 }
 
@@ -73,198 +75,123 @@ func (c *webhookClient) CreateRequest(ctx context.Context, method, url string, b
 	ctx, cancel := context.WithTimeout(ctx, 120*time.Second)
 	defer cancel()
 
-	// Retry logic with delays: 1s, 5s, 10s
-	retryDelays := []time.Duration{1 * time.Second, 5 * time.Second, 10 * time.Second}
-
-	var resp *http.Response
+	var (
+		resp    *http.Response
+		lastErr error
+	)
+
+	for i, delay := range c.retryDelays {
+		// Drain and close the previous iteration's body so the connection can be reused.
+		if resp != nil {
+			_, _ = io.Copy(io.Discard, resp.Body)
+			resp.Body.Close()
+			resp = nil
+		}
 
-	for i, delay := range retryDelays {
 		req, err := http.NewRequestWithContext(ctx, method, url, bytes.NewReader(bodyBytes))
 		if err != nil {
 			return nil, err
 		}
-
 		if c.Secret != nil {
 			req.Header.Set("X-Webhook-Secret", *c.Secret)
 		}
-
 		req.Header.Set("Content-Type", "application/json")
 
-		resp, err = c.httpClient.Do(req)
+		resp, lastErr = c.httpClient.Do(req)
 
-		if err == nil && resp != nil && resp.StatusCode >= 200 && resp.StatusCode < 300 {
+		// Don't retry on 2xx or permanent 4xx — only 408/429 are retryable in the 4xx range.
+		if lastErr == nil && resp.StatusCode < 500 &&
+			resp.StatusCode != http.StatusRequestTimeout &&
+			resp.StatusCode != http.StatusTooManyRequests {
 			return resp, nil
 		}
 
-		if i == len(retryDelays)-1 {
-			return nil, fmt.Errorf("webhook request failed with no response")
+		if i == len(c.retryDelays)-1 {
+			break
 		}
 
-		time.Sleep(delay)
+		select {
+		case <-ctx.Done():
+			if resp != nil {
+				_, _ = io.Copy(io.Discard, resp.Body)
+				resp.Body.Close()
+			}
+			return nil, ctx.Err()
+		case <-time.After(delay):
+		}
 	}
 
-	// This should never be reached due to the break condition above
-	return nil, fmt.Errorf("unexpected end of retry loop")
-
+	if lastErr != nil {
+		// http.Client.Do can return a non-nil response together with an error
+		// (e.g. CheckRedirect failures). Drain and close so the connection isn't leaked.
+		if resp != nil {
+			_, _ = io.Copy(io.Discard, resp.Body)
+			resp.Body.Close()
+		}
+		return nil, lastErr
+	}
+	return resp, nil
 }
 
-func (c *webhookClient) SendSBOM(ctx context.Context, SBOM cdx.BOM, org shared.OrgObject, project shared.ProjectObject, asset shared.AssetObject, assetVersion shared.AssetVersionObject, artifact shared.ArtifactObject) error {
-
+func (c *webhookClient) send(ctx context.Context, webhookType WebhookType, payload any, org shared.OrgObject, project shared.ProjectObject, asset shared.AssetObject, assetVersion shared.AssetVersionObject, artifact shared.ArtifactObject) error {
 	body := WebhookStruct{
 		Organization: org,
 		Project:      project,
 		Asset:        asset,
 		AssetVersion: assetVersion,
-		Payload:      SBOM,
-		Type:         WebhookTypeSBOM,
+		Payload:      payload,
+		Type:         webhookType,
 		Artifact:     artifact,
 	}
 
 	var buf bytes.Buffer
-	err := json.NewEncoder(&buf).Encode(body)
-	if err != nil {
+	if err := json.NewEncoder(&buf).Encode(body); err != nil {
 		return err
 	}
 
 	resp, err := c.CreateRequest(ctx, "POST", c.URL, &buf)
 	if err != nil {
 		return err
 	}
-	if resp == nil {
-		return fmt.Errorf("received nil response when sending SBOM")
-	}
 	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
-		return fmt.Errorf("failed to send SBOM, status: %s", resp.Status)
-	}
 
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return fmt.Errorf("webhook %s failed, status: %s", webhookType, resp.Status)
+	}
 	return nil
 }
 
-func (c *webhookClient) SendFirstPartyVulnerabilities(ctx context.Context, vuln []dtos.FirstPartyVulnDTO, org shared.OrgObject, project shared.ProjectObject, asset shared.AssetObject, assetVersion shared.AssetVersionObject) error {
-	return nil
-
-	/*body := WebhookStruct{
-		Organization: org,
-		Project:      project,
-		Asset:        asset,
-		AssetVersion: assetVersion,
-		Payload:      vuln,
-		Type:         WebhookTypeFirstPartyVulnerabilities,
-	}
-
-	var buf bytes.Buffer
-	err := json.NewEncoder(&buf).Encode(body)
-	if err != nil {
-		return err
-	}
-
-	resp, err := c.CreateRequest("POST", c.URL, &buf)
-	if err != nil {
-		return err
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("failed to send vulnerability, status: %s,", resp.Status)
-	}
+func (c *webhookClient) SendSBOM(ctx context.Context, SBOM cdx.BOM, org shared.OrgObject, project shared.ProjectObject, asset shared.AssetObject, assetVersion shared.AssetVersionObject, artifact shared.ArtifactObject) error {
+	return c.send(ctx, WebhookTypeSBOM, SBOM, org, project, asset, assetVersion, artifact)
+}
 
-	return nil*/
+func (c *webhookClient) SendFirstPartyVulnerabilities(ctx context.Context, vuln []dtos.FirstPartyVulnDTO, org shared.OrgObject, project shared.ProjectObject, asset shared.AssetObject, assetVersion shared.AssetVersionObject) error {
+	return c.send(ctx, WebhookTypeFirstPartyVulnerabilities, vuln, org, project, asset, assetVersion, shared.ArtifactObject{})
 }
 
 func (c *webhookClient) SendDependencyVulnerabilities(ctx context.Context, vuln []dtos.DependencyVulnDTO, org shared.OrgObject, project shared.ProjectObject, asset shared.AssetObject, assetVersion shared.AssetVersionObject, artifact shared.ArtifactObject) error {
-
-	body := WebhookStruct{
-		Organization: org,
-		Project:      project,
-		Asset:        asset,
-		AssetVersion: assetVersion,
-		Payload:      vuln,
-		Artifact:     artifact,
-		Type:         WebhookTypeDependencyVulnerabilities,
-	}
-
-	var buf bytes.Buffer
-	err := json.NewEncoder(&buf).Encode(body)
-	if err != nil {
-		return err
-	}
-
-	resp, err := c.CreateRequest(ctx, "POST", c.URL, &buf)
-	if err != nil {
-		return err
-	}
-	if resp == nil {
-		return fmt.Errorf("received nil response when sending dependency vulnerabilities")
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("failed to send vulnerability, status: %s", resp.Status)
-	}
-
-	return nil
+	return c.send(ctx, WebhookTypeDependencyVulnerabilities, vuln, org, project, asset, assetVersion, artifact)
 }
 
 func (c *webhookClient) SendTest(ctx context.Context, org shared.OrgObject, project shared.ProjectObject, asset shared.AssetObject, assetVersion shared.AssetVersionObject, payloadType TestPayloadType) error {
+	payload, webhookType := testPayload(payloadType)
+	return c.send(ctx, webhookType, payload, org, project, asset, assetVersion, shared.ArtifactObject{})
+}
 
-	var payload any
-	var webhookType WebhookType
-
+func testPayload(payloadType TestPayloadType) (any, WebhookType) {
 	switch payloadType {
-	case TestPayloadTypeEmpty:
-		payload = map[string]any{
-			"message":   "This is a test webhook from DevGuard",
-			"timestamp": time.Now().UTC().Format(time.RFC3339),
-		}
-		webhookType = WebhookTypeTest
-
 	case TestPayloadTypeSampleSBOM:
-		payload = createSampleSBOM()
-		webhookType = WebhookTypeSBOM
-
+		return createSampleSBOM(), WebhookTypeSBOM
 	case TestPayloadTypeSampleDependencyVulns:
-		payload = createSampleDependencyVulns()
-		webhookType = WebhookTypeDependencyVulnerabilities
-
+		return createSampleDependencyVulns(), WebhookTypeDependencyVulnerabilities
 	case TestPayloadTypeSampleFirstPartyVulns:
-		payload = createSampleFirstPartyVulns()
-		webhookType = WebhookTypeFirstPartyVulnerabilities
-
+		return createSampleFirstPartyVulns(), WebhookTypeFirstPartyVulnerabilities
 	default:
-		payload = map[string]any{
+		return map[string]any{
 			"message":   "This is a test webhook from DevGuard",
 			"timestamp": time.Now().UTC().Format(time.RFC3339),
-		}
-		webhookType = WebhookTypeTest
+		}, WebhookTypeTest
 	}
-
-	body := WebhookStruct{
-		Organization: org,
-		Project:      project,
-		Asset:        asset,
-		AssetVersion: assetVersion,
-		Payload:      payload,
-		Type:         webhookType,
-	}
-
-	var buf bytes.Buffer
-	err := json.NewEncoder(&buf).Encode(body)
-	if err != nil {
-		return err
-	}
-
-	resp, err := c.CreateRequest(ctx, "POST", c.URL, &buf)
-	if err != nil {
-		return err
-	}
-	if resp == nil {
-		return fmt.Errorf("received nil response when sending test webhook")
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
-		return nil // Success
-	}
-
-	return fmt.Errorf("failed to send test webhook, status: %s", resp.Status)
 }
 
 func createSampleSBOM() cdx.BOM {