fix multiple issues:

* wrong action name: stack_trace -> generate_stack
* stacktrace msgp serialization error: wait for new version for better support
* dyngo.EmitData on parent operation instead of child operation because of variable shadowing
* update contrib roundtripper to ignore events.SecurityBlockingEvent

Signed-off-by: Eliott Bouhana <[email protected]>

contrib/net/http/roundtripper.go

@@ -6,12 +6,14 @@
 package http
 
 import (
+	"errors"
 	"fmt"
 	"math"
 	"net/http"
 	"os"
 	"strconv"
 
+	"gopkg.in/DataDog/dd-trace-go.v1/appsec/events"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
@@ -59,7 +61,7 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er
 		if rt.cfg.after != nil {
 			rt.cfg.after(res, span)
 		}
-		if rt.cfg.errCheck == nil || rt.cfg.errCheck(err) {
+		if !errors.Is(err, &events.SecurityBlockingEvent{}) && (rt.cfg.errCheck == nil || rt.cfg.errCheck(err)) {
 			span.Finish(tracer.WithError(err))
 		} else {
 			span.Finish()
@@ -77,11 +79,15 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er
 			fmt.Fprintf(os.Stderr, "contrib/net/http.Roundtrip: failed to inject http headers: %v\n", err)
 		}
 	}
+
 	if appsec.Enabled() {
-		res, err = httpsec.RoundTrip(ctx, r2, rt.base)
-	} else {
-		res, err = rt.base.RoundTrip(r2)
+		if err := httpsec.ProtectRoundTrip(ctx, r2.URL.String()); err != nil {
+			return nil, err
+		}
 	}
+
+	res, err = rt.base.RoundTrip(r2)
+
 	if err != nil {
 		span.SetTag("http.errors", err.Error())
 		if rt.cfg.errCheck == nil || rt.cfg.errCheck(err) {

contrib/net/http/roundtripper_test.go

@@ -8,7 +8,6 @@ package http
 import (
 	"encoding/base64"
 	"fmt"
-	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/httpsec"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -17,12 +16,14 @@ import (
 	"testing"
 	"time"
 
+	"gopkg.in/DataDog/dd-trace-go.v1/appsec/events"
 	"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/namingschematest"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec"
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/listener/httpsec"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig"
 
 	"github.com/stretchr/testify/assert"
@@ -653,8 +654,10 @@ func TestAppsec(t *testing.T) {
 			require.NoError(t, err)
 
 			resp, err := client.RoundTrip(req.WithContext(r.Context()))
-			defer require.NoError(t, err)
-			defer resp.Body.Close()
+			require.ErrorIs(t, err, &events.SecurityBlockingEvent{})
+			if resp != nil {
+				defer resp.Body.Close()
+			}
 		}), w, r, &ServeConfig{
 			Service:  "service",
 			Resource: "resource",
@@ -665,10 +668,11 @@ func TestAppsec(t *testing.T) {
 		serviceSpan := spans[1]
 
 		require.Contains(t, serviceSpan.Tags(), "_dd.appsec.json")
-
 		appsecJSON := serviceSpan.Tag("_dd.appsec.json")
 		require.Contains(t, appsecJSON, httpsec.ServerIoNetURLAddr)
 
+		require.Contains(t, serviceSpan.Tags(), "_dd.stack")
+
 		// This is a nested event so it should contain the child span id in the service entry span
 		// TODO(eliott.bouhana): uncomment this once we have the child span id in the service entry span
 		// require.Contains(t, appsecJSON, `"span_id":`+strconv.FormatUint(requestSpan.SpanID(), 10))

internal/appsec/emitter/httpsec/roundtripper.go

@@ -7,7 +7,6 @@ package httpsec
 
 import (
 	"context"
-	"net/http"
 	"sync"
 
 	"gopkg.in/DataDog/dd-trace-go.v1/appsec/events"
@@ -19,8 +18,7 @@ import (
 
 var badInputContextOnce sync.Once
 
-func RoundTrip(ctx context.Context, request *http.Request, rt http.RoundTripper) (*http.Response, error) {
-	url := request.URL.String()
+func ProtectRoundTrip(ctx context.Context, url string) error {
 	opArgs := types.RoundTripOperationArgs{
 		URL: url,
 	}
@@ -32,7 +30,7 @@ func RoundTrip(ctx context.Context, request *http.Request, rt http.RoundTripper)
 				"instrumentation metadata in the request context: the request handler is not being monitored by a " +
 				"middleware function or the incoming request context has not be forwarded correctly to the roundtripper")
 		})
-		return rt.RoundTrip(request)
+		return nil
 	}
 
 	op := &types.RoundTripOperation{
@@ -49,8 +47,8 @@ func RoundTrip(ctx context.Context, request *http.Request, rt http.RoundTripper)
 
 	if err != nil {
 		log.Debug("appsec: outgoing http request blocked by the WAF on URL: %s", url)
-		return nil, err
+		return err
 	}
 
-	return rt.RoundTrip(request)
+	return nil
 }

internal/appsec/listener/httpsec/roundtripper.go

@@ -17,7 +17,7 @@ import (
 
 // RegisterRoundTripperListener registers a listener on outgoing requests to run the WAF.
 func RegisterRoundTripperListener(op dyngo.Operation, events *trace.SecurityEventsHolder, wafCtx *waf.Context, limiter limiter.Limiter) {
-	dyngo.On(op, func(operation *types.RoundTripOperation, args types.RoundTripOperationArgs) {
+	dyngo.On(op, func(op *types.RoundTripOperation, args types.RoundTripOperationArgs) {
 		wafResult := sharedsec.RunWAF(wafCtx, waf.RunAddressData{Persistent: map[string]any{ServerIoNetURLAddr: args.URL}})
 		if !wafResult.HasEvents() {
 			return

internal/appsec/listener/sharedsec/shared.go

@@ -79,19 +79,24 @@ func AddWAFMonitoringTags(th trace.TagSetter, rulesVersion string, stats map[str
 // When SDKError is not nil, this error is sent to the op with EmitData so that the invoked SDK can return it
 func ProcessActions(op dyngo.Operation, actions map[string]any) (interrupt bool) {
 	for aType, params := range actions {
+		log.Debug("appsec: processing %s action with params %v", aType, params)
 		actionArray := ActionsFromEntry(aType, params)
 		if actionArray == nil {
 			log.Debug("cannot process %s action with params %v", aType, params)
 			continue
 		}
 		for _, a := range actionArray {
 			a.EmitData(op)
-			if a.Blocking() { // Send the error to be returned by the SDK
-				interrupt = true
-				dyngo.EmitData(op, &events.SecurityBlockingEvent{}) // Send error
-			}
+			interrupt = interrupt || a.Blocking()
 		}
 	}
+
+	// If any of the actions are supposed to interrupt the request, emit a blocking event for the SDK operations
+	// to return an error.
+	if interrupt {
+		dyngo.EmitData(op, &events.SecurityBlockingEvent{})
+	}
+
 	return interrupt
 }
 

internal/appsec/testdata/rasp.json

@@ -49,7 +49,8 @@
             ],
             "transformers": [],
             "on_match": [
-                "stack_trace"
+                "stack_trace",
+                "block"
             ]
         },
         {
@@ -97,7 +98,8 @@
             ],
             "transformers": [],
             "on_match": [
-                "stack_trace"
+                "stack_trace",
+                "block"
             ]
         },
         {
@@ -147,7 +149,8 @@
             ],
             "transformers": [],
             "on_match": [
-                "stack_trace"
+                "stack_trace",
+                "block"
             ]
         }
     ],

internal/stacktrace/event.go

@@ -88,14 +88,16 @@ func AddToSpan(span ddtrace.Span, events ...*Event) {
 		return
 	}
 
-	groupByCategory := map[EventCategory][]*Event{
-		ExceptionEvent:     {},
-		VulnerabilityEvent: {},
-		ExploitEvent:       {},
-	}
+	// TODO(eliott.bouhana): switch to a map[EventCategory][]*Event type when the tinylib/[email protected] is out
+	groupByCategory := make(map[string]any, 3)
 
 	for _, event := range events {
-		groupByCategory[event.Category] = append(groupByCategory[event.Category], event)
+		if _, ok := groupByCategory[string(event.Category)]; !ok {
+			groupByCategory[string(event.Category)] = []*Event{event}
+			continue
+		}
+
+		groupByCategory[string(event.Category)] = append(groupByCategory[string(event.Category)].([]*Event), event)
 	}
 
 	type rooter interface {

internal/stacktrace/event_test.go

@@ -6,6 +6,7 @@
 package stacktrace
 
 import (
+	"github.com/tinylib/msgp/msgp"
 	"testing"
 
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer"
@@ -38,11 +39,29 @@ func TestEventToSpan(t *testing.T) {
 	require.Len(t, spans, 1)
 	require.Equal(t, "op", spans[0].OperationName())
 
-	eventsMap := spans[0].Tag("_dd.stack").(internal.MetaStructValue).Value.(map[EventCategory][]*Event)
-	require.Len(t, eventsMap, 3)
+	eventsMap := spans[0].Tag("_dd.stack").(internal.MetaStructValue).Value.(map[string]any)
+	require.Len(t, eventsMap, 1)
 
-	eventsCat := eventsMap[ExceptionEvent]
+	eventsCat := eventsMap[string(ExceptionEvent)].([]*Event)
 	require.Len(t, eventsCat, 1)
 
 	require.Equal(t, *event, *eventsCat[0])
 }
+
+func TestMsgPackSerialization(t *testing.T) {
+	mt := mocktracer.Start()
+	defer mt.Stop()
+
+	span := ddtracer.StartSpan("op")
+	event := NewEvent(ExceptionEvent, WithMessage("message"), WithType("type"), WithID("id"))
+	AddToSpan(span, event)
+	span.Finish()
+
+	spans := mt.FinishedSpans()
+	require.Len(t, spans, 1)
+
+	eventsMap := spans[0].Tag("_dd.stack").(internal.MetaStructValue).Value
+
+	_, err := msgp.AppendIntf(nil, eventsMap)
+	require.NoError(t, err)
+}