From d13e7e885c26e7ddc2294ce5665d3e7cc8970d69 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 06:54:39 +0000 Subject: [PATCH] Send the build version in the outbound User-Agent (closes #313) The http and slack targets sent the fixed User-Agent webhooker/1.0. The delivery engine now takes the version from Globals, the value the web UI footer shows, and one method builds webhooker/ for both targets. The http target still sets it after the configured headers, so a configured or inbound User-Agent never reaches the wire. A test builds the engine through its real constructor with a known version and checks the header each target sends. Model: opus-5-5 --- internal/delivery/engine.go | 14 +++++ internal/delivery/engine_test.go | 6 +- internal/delivery/export_test.go | 3 +- internal/delivery/redirect_test.go | 1 + internal/delivery/target_http.go | 9 ++- internal/delivery/target_slack.go | 2 +- internal/delivery/user_agent_test.go | 91 ++++++++++++++++++++++++++++ 7 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 internal/delivery/user_agent_test.go diff --git a/internal/delivery/engine.go b/internal/delivery/engine.go index 7d78650..6fae09d 100644 --- a/internal/delivery/engine.go +++ b/internal/delivery/engine.go @@ -14,6 +14,7 @@ import ( "go.uber.org/fx" "gorm.io/gorm" "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/globals" "sneak.berlin/go/webhooker/internal/lifecycle" "sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/metrics" @@ -146,6 +147,7 @@ type EngineParams struct { DB *database.Database DBManager *database.WebhookDBManager + Globals *globals.Globals Logger *logger.Logger SSRFGuard *Guard Metrics *metrics.Set @@ -168,6 +170,10 @@ type Engine struct { retryCh chan Task workers int + // version is the running build's version, the one the web UI + // footer shows. userAgent puts it on every outbound request. + version string + // mtr is the delivery metric set. Production wires the one // registered on the registry /metrics serves; a test can // substitute a set registered on a registry it holds, so it can @@ -205,6 +211,7 @@ func New( deliveryCh: make(chan Task, deliveryChannelSize), retryCh: make(chan Task, retryChannelSize), workers: defaultWorkers, + version: params.Globals.Version, mtr: params.Metrics, } @@ -301,6 +308,13 @@ func (e *Engine) ScheduleRetry( }) } +// userAgent is the User-Agent header of every http and slack +// delivery request: the program name and the running build's +// version. +func (e *Engine) userAgent() string { + return "webhooker/" + e.version +} + // registerHooks wires the engine's start and stop into the fx // lifecycle. The start hook's context is deliberately ignored // (see start for why the worker pool must not inherit it); the diff --git a/internal/delivery/engine_test.go b/internal/delivery/engine_test.go index abe668b..676b12b 100644 --- a/internal/delivery/engine_test.go +++ b/internal/delivery/engine_test.go @@ -1247,11 +1247,6 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) { testContentType, receivedHeaders.Get("Content-Type"), ) - - assert.Equal(t, - "webhooker/1.0", - receivedHeaders.Get("User-Agent"), - ) } // The event's stored inbound headers carry the same Content-Type the @@ -1320,6 +1315,7 @@ func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) { ContentType: tc.event, }, cfg, + "webhooker/dev", ) assert.Equal(t, diff --git a/internal/delivery/export_test.go b/internal/delivery/export_test.go index a037027..a769f54 100644 --- a/internal/delivery/export_test.go +++ b/internal/delivery/export_test.go @@ -83,8 +83,9 @@ func ExportApplyRequestHeaders( req *http.Request, event *database.Event, cfg *HTTPTargetConfig, + userAgent string, ) []string { - return applyRequestHeaders(req, event, cfg) + return applyRequestHeaders(req, event, cfg, userAgent) } // ExportTruncate exposes truncate for testing. diff --git a/internal/delivery/redirect_test.go b/internal/delivery/redirect_test.go index e5d5347..507327f 100644 --- a/internal/delivery/redirect_test.go +++ b/internal/delivery/redirect_test.go @@ -375,6 +375,7 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) { "Content-Type": testContentType, }, }, + "webhooker/dev", ) assert.Equal(t, diff --git a/internal/delivery/target_http.go b/internal/delivery/target_http.go index 9c7b539..1f699f8 100644 --- a/internal/delivery/target_http.go +++ b/internal/delivery/target_http.go @@ -442,7 +442,9 @@ func (t *httpTarget) doHTTPRequest( ) } - originScoped := applyRequestHeaders(req, event, cfg) + originScoped := applyRequestHeaders( + req, event, cfg, t.eng.userAgent(), + ) client := t.clientForRequest(cfg, originScoped) @@ -562,10 +564,13 @@ func isForwardableHeader(name string) bool { // Content-Type goes out once: a Content-Type configured on the target // wins, otherwise the event's ContentType, otherwise none. The inbound // Content-Type in the event's headers is never forwarded. +// +// userAgent is set last, over any configured or inbound User-Agent. func applyRequestHeaders( req *http.Request, event *database.Event, cfg *HTTPTargetConfig, + userAgent string, ) []string { if event.ContentType != "" { req.Header.Set( @@ -580,7 +585,7 @@ func applyRequestHeaders( originScoped[http.CanonicalHeaderKey(k)] = struct{}{} } - req.Header.Set("User-Agent", "webhooker/1.0") + req.Header.Set("User-Agent", userAgent) // A Content-Type configured on the target describes the body // being sent rather than the sender. A 307/308 preserves the diff --git a/internal/delivery/target_slack.go b/internal/delivery/target_slack.go index c2f7ff7..3703b89 100644 --- a/internal/delivery/target_slack.go +++ b/internal/delivery/target_slack.go @@ -136,7 +136,7 @@ func (t *slackTarget) attempt( } req.Header.Set("Content-Type", "application/json") - req.Header.Set("User-Agent", "webhooker/1.0") + req.Header.Set("User-Agent", t.eng.userAgent()) resp, doErr := executeHTTPRequest(t.client, req) durationMs := time.Since(start).Milliseconds() diff --git a/internal/delivery/user_agent_test.go b/internal/delivery/user_agent_test.go new file mode 100644 index 0000000..2ff6f3c --- /dev/null +++ b/internal/delivery/user_agent_test.go @@ -0,0 +1,91 @@ +package delivery_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/netip" + "testing" + + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/fx/fxtest" + "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/delivery" + "sneak.berlin/go/webhooker/internal/globals" + "sneak.berlin/go/webhooker/internal/logger" + "sneak.berlin/go/webhooker/internal/metrics" +) + +// Both the http and the slack target send webhooker/ and the version +// in Globals, the value the web UI footer shows. A User-Agent +// configured on the target or carried in by the sender does not +// replace it. +func TestUserAgent_IsTheBuildVersion(t *testing.T) { + t.Parallel() + + const want = "webhooker/1.2.3-test" + + userAgents := make(chan string, 1) + + ts := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + userAgents <- r.Header.Get("User-Agent") + + w.WriteHeader(http.StatusOK) + }, + )) + defer ts.Close() + + g := &globals.Globals{Version: "1.2.3-test"} + lc := fxtest.NewLifecycle(t) + + log, err := logger.New(lc, logger.LoggerParams{Globals: g}) + require.NoError(t, err) + + e := delivery.New(lc, delivery.EngineParams{ + Globals: g, + Logger: log, + // httptest listens on loopback, which the default guard + // refuses. + SSRFGuard: delivery.NewTestGuard( + netip.MustParsePrefix("127.0.0.0/8"), + ), + Metrics: metrics.New(prometheus.NewRegistry()), + }) + + statusCode, _, _, err := e.ExportDoHTTPRequest( + context.Background(), + &delivery.HTTPTargetConfig{ + URL: ts.URL, + Headers: map[string]string{"User-Agent": "configured/1"}, + }, + &database.Event{Headers: `{"User-Agent":["curl/8"]}`}, + ) + require.NoError(t, err) + require.Equal(t, http.StatusOK, statusCode) + require.Len(t, userAgents, 1, "the http target sent no request") + assert.Equal(t, want, <-userAgents, "http target") + + db := testWebhookDB(t) + targetID := uuid.New().String() + + slackCfg, err := json.Marshal( + delivery.SlackTargetConfig{WebhookURL: ts.URL}, + ) + require.NoError(t, err) + + event := seedEvent(t, db, `{"action":"test"}`) + dlv := seedDelivery( + t, db, event.ID, targetID, database.DeliveryStatusPending, + ) + + e.ExportDeliverSlack(context.Background(), db, buildSlackDelivery( + dlv, event, targetID, "test-slack", string(slackCfg), + )) + require.Len(t, userAgents, 1, "the slack target sent no request") + assert.Equal(t, want, <-userAgents, "slack target") +}