fix: preserve webhook URL query params in generic notification provider (#2292)

backend/pkg/utils/notifications/generic_sender.go

@@ -42,58 +42,50 @@ func BuildGenericURL(config models.GenericConfig) (string, error) {
 
 	// Build generic service URL
 	// Format: generic://host[:port]/path?params
-	// Shoutrrr's generic service uses HTTP or HTTPS based on the DisableTLS setting
-	scheme := "generic"
-
-	// Start with the base URL.
-	// Preserve any query parameters from the original webhook URL by encoding
-	// the ? as %3F so it stays in the path component. Shoutrrr decodes the path
-	// when constructing the outbound HTTP request, recovering the original query
-	// string for the upstream service.
-	rawPath := webhookURL.Path
-	decodedPath := webhookURL.Path
-	if webhookURL.RawQuery != "" {
-		rawPath = webhookURL.Path + "%3F" + webhookURL.RawQuery
-		decodedPath = webhookURL.Path + "?" + webhookURL.RawQuery
-	}
-	shoutrrrURL := &url.URL{
-		Scheme:  scheme,
-		Host:    webhookURL.Host,
-		RawPath: rawPath,
-		Path:    decodedPath,
-	}
-
-	// Build query parameters
-	query := url.Values{}
-
-	// Set template to JSON (default for generic webhooks)
-	query.Set("template", "json")
-
-	// Set content type if provided
-	if config.ContentType != "" {
-		query.Set("contenttype", config.ContentType)
-	}
-
-	// Set HTTP method if provided
-	if config.Method != "" {
-		query.Set("method", config.Method)
-	}
-
-	// Set title and message keys if provided
-	if config.TitleKey != "" {
-		query.Set("titlekey", config.TitleKey)
-	}
-	if config.MessageKey != "" {
-		query.Set("messagekey", config.MessageKey)
-	}
-
-	// Determine TLS setting from the webhook URL scheme (http/https)
-	// If scheme is missing, DisableTLS is only used to infer the default scheme above.
+	// Shoutrrr's generic service uses HTTP or HTTPS based on the DisableTLS setting.
+
+	// Start from the user's existing query parameters. Shoutrrr's generic
+	// service preserves any query keys it does not recognise, so provider
+	// tokens embedded in the webhook URL (e.g. PushPlus's `?token=...`) flow
+	// straight through to the outbound HTTP request untouched.
+	//
+	// For Shoutrrr config keys (template, contenttype, method, titlekey,
+	// messagekey, disabletls) we only fill in defaults / configured values
+	// when the user has not already set the same key inline in the URL.
+	// That way an explicit `?template=custom` or `?disabletls=yes` from the
+	// user is always respected and never silently overwritten by the
+	// provider settings or the URL-scheme-derived TLS flag.
+	query := webhookURL.Query()
+
+	setDefault := func(key, value string) {
+		if value == "" {
+			return
+		}
+		if query.Get(key) != "" {
+			return
+		}
+		query.Set(key, value)
+	}
+
+	// Default to the JSON template — Shoutrrr's JSON template marshals the
+	// notification params as a flat JSON object at the root level, which is
+	// the format most providers (PushPlus, custom APIs, Home Assistant, etc.)
+	// expect.
+	setDefault("template", "json")
+	setDefault("contenttype", config.ContentType)
+	setDefault("method", config.Method)
+	setDefault("titlekey", config.TitleKey)
+	setDefault("messagekey", config.MessageKey)
+
+	// Determine TLS setting from the webhook URL scheme (http/https) when the
+	// user has not already passed `disabletls` explicitly. If the scheme is
+	// missing here we treat it as a hard error because Shoutrrr needs an
+	// explicit transport.
 	switch strings.ToLower(webhookURL.Scheme) {
 	case "http":
-		query.Set("disabletls", "yes")
+		setDefault("disabletls", "yes")
 	case "https":
-		query.Set("disabletls", "no")
+		setDefault("disabletls", "no")
 	default:
 		return "", fmt.Errorf("invalid webhook URL scheme: %s", webhookURL.Scheme)
 	}
@@ -106,7 +98,12 @@ func BuildGenericURL(config models.GenericConfig) (string, error) {
 		}
 	}
 
-	shoutrrrURL.RawQuery = query.Encode()
+	shoutrrrURL := &url.URL{
+		Scheme:   "generic",
+		Host:     webhookURL.Host,
+		Path:     webhookURL.Path,
+		RawQuery: query.Encode(),
+	}
 
 	return shoutrrrURL.String(), nil
 }

backend/pkg/utils/notifications/generic_sender_test.go

@@ -134,14 +134,28 @@ func TestBuildGenericURL(t *testing.T) {
 			config: models.GenericConfig{
 				WebhookURL: "http://www.pushplus.plus/send?token=abc123",
 			},
-			wantURL: "generic://www.pushplus.plus/send%3Ftoken=abc123?disabletls=yes&template=json",
+			wantURL: "generic://www.pushplus.plus/send?disabletls=yes&template=json&token=abc123",
 		},
 		{
 			name: "webhook URL with multiple query params preserved",
 			config: models.GenericConfig{
 				WebhookURL: "https://api.example.com/webhook?token=abc&channel=general",
 			},
-			wantURL: "generic://api.example.com/webhook%3Ftoken=abc&channel=general?disabletls=no&template=json",
+			wantURL: "generic://api.example.com/webhook?channel=general&disabletls=no&template=json&token=abc",
+		},
+		{
+			name: "PushPlus webhook with content message key",
+			config: models.GenericConfig{
+				WebhookURL: "http://www.pushplus.plus/send?token=abc123",
+				Method:     "POST",
+				MessageKey: "content",
+			},
+			// Shoutrrr's generic service preserves `token=abc123` through to the
+			// outbound HTTP request untouched, while consuming the config keys
+			// (disabletls, template, method, messagekey). This is what PushPlus
+			// needs: POST http://www.pushplus.plus/send?token=abc123 with
+			// {"title":"...","content":"..."} at the root.
+			wantURL: "generic://www.pushplus.plus/send?disabletls=yes&messagekey=content&method=POST&template=json&token=abc123",
 		},
 		{
 			name: "empty webhook URL",
@@ -338,3 +352,65 @@ func TestBuildGenericURL_CustomKeys(t *testing.T) {
 		})
 	}
 }
+
+// TestBuildGenericURL_PreservesUserShoutrrrConfigKeys verifies that an
+// explicit Shoutrrr config key embedded by the user in the webhook URL is
+// never silently overwritten by the provider defaults, the configured field
+// values, or the URL-scheme-derived TLS flag.
+func TestBuildGenericURL_PreservesUserShoutrrrConfigKeys(t *testing.T) {
+	tests := []struct {
+		name      string
+		config    models.GenericConfig
+		wantInURL []string
+		notInURL  []string
+	}{
+		{
+			name: "user template wins over default json",
+			config: models.GenericConfig{
+				WebhookURL: "https://example.com/api?template=custom",
+			},
+			wantInURL: []string{"template=custom"},
+			notInURL:  []string{"template=json"},
+		},
+		{
+			name: "user disabletls wins over scheme-derived value",
+			config: models.GenericConfig{
+				WebhookURL: "https://example.com/api?disabletls=yes",
+			},
+			wantInURL: []string{"disabletls=yes"},
+			notInURL:  []string{"disabletls=no"},
+		},
+		{
+			name: "user messagekey wins over configured value",
+			config: models.GenericConfig{
+				WebhookURL: "https://example.com/api?messagekey=user_msg",
+				MessageKey: "configured_msg",
+			},
+			wantInURL: []string{"messagekey=user_msg"},
+			notInURL:  []string{"messagekey=configured_msg"},
+		},
+		{
+			name: "user method wins over configured value",
+			config: models.GenericConfig{
+				WebhookURL: "https://example.com/api?method=PUT",
+				Method:     "POST",
+			},
+			wantInURL: []string{"method=PUT"},
+			notInURL:  []string{"method=POST"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotURL, err := BuildGenericURL(tt.config)
+			require.NoError(t, err)
+
+			for _, want := range tt.wantInURL {
+				assert.Contains(t, gotURL, want)
+			}
+			for _, notWant := range tt.notInURL {
+				assert.NotContains(t, gotURL, notWant)
+			}
+		})
+	}
+}