fix: enhance Discord webhook validation and improve error handling for IP address hosts

This commit is contained in:
GitHub Actions
2026-02-14 15:15:34 +00:00
parent 380a0ab60f
commit ee72fc8f65
6 changed files with 202 additions and 148 deletions

View File

@@ -486,8 +486,7 @@ COPY scripts/ /app/scripts/
RUN chmod +x /app/scripts/db-recovery.sh
# Set default environment variables
ENV GODEBUG=netdns=go \
CHARON_ENV=production \
ENV CHARON_ENV=production \
CHARON_DB_PATH=/app/data/charon.db \
CHARON_FRONTEND_DIR=/app/frontend/dist \
CHARON_CADDY_ADMIN_API=http://localhost:2019 \

View File

@@ -48,7 +48,7 @@ func (h *NotificationProviderHandler) Create(c *gin.Context) {
if err := h.service.CreateProvider(&provider); err != nil {
// If it's a validation error from template parsing, return 400
if strings.Contains(err.Error(), "invalid custom template") || strings.Contains(err.Error(), "rendered template") || strings.Contains(err.Error(), "failed to parse template") || strings.Contains(err.Error(), "failed to render template") {
if isProviderValidationError(err) {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
@@ -75,7 +75,7 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) {
provider.ID = id
if err := h.service.UpdateProvider(&provider); err != nil {
if strings.Contains(err.Error(), "invalid custom template") || strings.Contains(err.Error(), "rendered template") || strings.Contains(err.Error(), "failed to parse template") || strings.Contains(err.Error(), "failed to render template") {
if isProviderValidationError(err) {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
@@ -88,6 +88,19 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) {
c.JSON(http.StatusOK, provider)
}
func isProviderValidationError(err error) bool {
if err == nil {
return false
}
errMsg := err.Error()
return strings.Contains(errMsg, "invalid custom template") ||
strings.Contains(errMsg, "rendered template") ||
strings.Contains(errMsg, "failed to parse template") ||
strings.Contains(errMsg, "failed to render template") ||
strings.Contains(errMsg, "invalid Discord webhook URL")
}
func (h *NotificationProviderHandler) Delete(c *gin.Context) {
if !requireAdmin(c) {
return

View File

@@ -232,3 +232,37 @@ func TestNotificationProviderHandler_Preview(t *testing.T) {
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusBadRequest, w.Code)
}
func TestNotificationProviderHandler_CreateRejectsDiscordIPHost(t *testing.T) {
r, _ := setupNotificationProviderTest(t)
provider := models.NotificationProvider{
Name: "Discord IP",
Type: "discord",
URL: "https://203.0.113.10/api/webhooks/123456/token_abc",
}
body, _ := json.Marshal(provider)
req, _ := http.NewRequest("POST", "/api/v1/notifications/providers", bytes.NewBuffer(body))
w := httptest.NewRecorder()
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Contains(t, w.Body.String(), "invalid Discord webhook URL")
assert.Contains(t, w.Body.String(), "IP address hosts are not allowed")
}
func TestNotificationProviderHandler_CreateAcceptsDiscordHostname(t *testing.T) {
r, _ := setupNotificationProviderTest(t)
provider := models.NotificationProvider{
Name: "Discord Host",
Type: "discord",
URL: "https://discord.com/api/webhooks/123456/token_abc",
}
body, _ := json.Marshal(provider)
req, _ := http.NewRequest("POST", "/api/v1/notifications/providers", bytes.NewBuffer(body))
w := httptest.NewRecorder()
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusCreated, w.Code)
}

View File

@@ -34,6 +34,11 @@ func NewNotificationService(db *gorm.DB) *NotificationService {
var discordWebhookRegex = regexp.MustCompile(`^https://discord(?:app)?\.com/api/webhooks/(\d+)/([a-zA-Z0-9_-]+)`)
var allowedDiscordWebhookHosts = map[string]struct{}{
"discord.com": {},
"canary.discord.com": {},
}
func normalizeURL(serviceType, rawURL string) string {
if serviceType == "discord" {
matches := discordWebhookRegex.FindStringSubmatch(rawURL)
@@ -46,33 +51,42 @@ func normalizeURL(serviceType, rawURL string) string {
return rawURL
}
func canonicalizeDiscordWebhookURL(providerType, rawURL string) string {
if !strings.EqualFold(providerType, "discord") {
return rawURL
}
func validateDiscordWebhookURL(rawURL string) error {
parsedURL, err := neturl.Parse(rawURL)
if err != nil {
return rawURL
return fmt.Errorf("invalid Discord webhook URL: failed to parse URL; use the HTTPS webhook URL provided by Discord")
}
if strings.EqualFold(parsedURL.Scheme, "discord") {
return nil
}
if !strings.EqualFold(parsedURL.Scheme, "https") {
return rawURL
return fmt.Errorf("invalid Discord webhook URL: URL must use HTTPS and the hostname URL provided by Discord")
}
hostname := parsedURL.Hostname()
if net.ParseIP(hostname) == nil {
return rawURL
hostname := strings.ToLower(parsedURL.Hostname())
if hostname == "" {
return fmt.Errorf("invalid Discord webhook URL: missing hostname; use the HTTPS webhook URL provided by Discord")
}
port := parsedURL.Port()
if port == "" || port == "443" {
parsedURL.Host = "discord.com"
} else {
parsedURL.Host = net.JoinHostPort("discord.com", port)
if net.ParseIP(hostname) != nil {
return fmt.Errorf("invalid Discord webhook URL: IP address hosts are not allowed; use the hostname URL provided by Discord (discord.com or canary.discord.com)")
}
return parsedURL.String()
if _, ok := allowedDiscordWebhookHosts[hostname]; !ok {
return fmt.Errorf("invalid Discord webhook URL: host must be discord.com or canary.discord.com; use the hostname URL provided by Discord")
}
return nil
}
func validateDiscordProviderURL(providerType, rawURL string) error {
if !strings.EqualFold(providerType, "discord") {
return nil
}
return validateDiscordWebhookURL(rawURL)
}
// supportsJSONTemplates returns true if the provider type can use JSON templates
@@ -196,6 +210,12 @@ func (s *NotificationService) SendExternal(ctx context.Context, eventType, title
// In production it defaults to shoutrrr.Send.
var shoutrrrSendFunc = shoutrrr.Send
// webhookDoRequestFunc is a test hook for outbound JSON webhook requests.
// In production it defaults to (*http.Client).Do.
var webhookDoRequestFunc = func(client *http.Client, req *http.Request) (*http.Response, error) {
return client.Do(req)
}
func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.NotificationProvider, data map[string]any) error {
// Built-in templates
const minimalTemplate = `{"message": {{toJSON .Message}}, "title": {{toJSON .Title}}, "time": {{toJSON .Time}}, "event": {{toJSON .EventType}}}`
@@ -234,7 +254,11 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti
// Additionally, we apply `isValidRedirectURL` as a barrier-guard style predicate.
// CodeQL recognizes this pattern as a sanitizer for untrusted URL values, while
// the real SSRF protection remains `security.ValidateExternalURL`.
webhookURL := canonicalizeDiscordWebhookURL(p.Type, p.URL)
if err := validateDiscordProviderURL(p.Type, p.URL); err != nil {
return err
}
webhookURL := p.URL
if !isValidRedirectURL(webhookURL) {
return fmt.Errorf("invalid webhook url")
@@ -322,81 +346,7 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti
network.WithAllowLocalhost(), // Allow localhost for testing
)
// Resolve the hostname to an explicit IP and construct the request URL using the
// resolved IP. This prevents direct user-controlled hostnames from being used
// as the request's destination (SSRF mitigation) and helps CodeQL validate the
// sanitisation performed by security.ValidateExternalURL.
//
// NOTE (security): The following mitigations are intentionally applied to
// reduce SSRF/request-forgery risk:
// - security.ValidateExternalURL enforces http(s) schemes and rejects private IPs
// (except explicit localhost for testing) after DNS resolution.
// - We perform an additional DNS resolution here and choose a non-private
// IP to use as the TCP destination to avoid direct hostname-based routing.
// - We set the request's `Host` header to the original hostname so virtual
// hosting works while the actual socket connects to a resolved IP.
// - The HTTP client disables automatic redirects and has a short timeout.
// Together these steps make the request destination unambiguous and prevent
// accidental requests to internal networks. If your threat model requires
// stricter controls, consider an explicit allowlist of webhook hostnames.
// Re-parse the validated URL string to get hostname for DNS lookup.
// This uses the sanitized string rather than the original tainted input.
validatedURL, _ := neturl.Parse(validatedURLStr)
// Normalize scheme to a constant value derived from an allowlisted set.
// This avoids propagating the original input string directly into request construction.
var safeScheme string
switch validatedURL.Scheme {
case "http":
safeScheme = "http"
case "https":
safeScheme = "https"
default:
return fmt.Errorf("invalid webhook url: unsupported scheme")
}
ips, err := net.LookupIP(validatedURL.Hostname())
if err != nil || len(ips) == 0 {
return fmt.Errorf("failed to resolve webhook host: %w", err)
}
// If hostname is local loopback, accept loopback addresses; otherwise pick
// the first non-private IP (security.ValidateExternalURL already ensured these
// are not private, but check again defensively).
var selectedIP net.IP
for _, ip := range ips {
if validatedURL.Hostname() == "localhost" || validatedURL.Hostname() == "127.0.0.1" || validatedURL.Hostname() == "::1" {
selectedIP = ip
break
}
if !isPrivateIP(ip) {
selectedIP = ip
break
}
}
if selectedIP == nil {
return fmt.Errorf("failed to find non-private IP for webhook host: %s", validatedURL.Hostname())
}
port := validatedURL.Port()
if port == "" {
if safeScheme == "https" {
port = "443"
} else {
port = "80"
}
}
// Construct a safe URL using the resolved IP:port for the Host component,
// while preserving the original path and query from the validated URL.
// This makes the destination hostname unambiguously an IP that we resolved
// and prevents accidental requests to private/internal addresses.
// Using validatedURL (derived from validatedURLStr) breaks the CodeQL taint chain.
safeURL := &neturl.URL{
Scheme: safeScheme,
Host: net.JoinHostPort(selectedIP.String(), port),
Path: validatedURL.Path,
RawQuery: validatedURL.RawQuery,
}
req, err := http.NewRequestWithContext(ctx, "POST", safeURL.String(), &body)
req, err := http.NewRequestWithContext(ctx, "POST", validatedURLStr, &body)
if err != nil {
return fmt.Errorf("failed to create webhook request: %w", err)
}
@@ -407,22 +357,15 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti
req.Header.Set("X-Request-ID", ridStr)
}
}
// Preserve original hostname for virtual host (Host header)
// Using validatedURL.Host ensures we're using the sanitized value.
req.Host = validatedURL.Host
// We validated the URL and resolved the hostname to an explicit IP above.
// The request uses the resolved IP (selectedIP) and we also set the
// Host header to the original hostname, so virtual-hosting works while
// preventing requests to private or otherwise disallowed addresses.
// This mitigates SSRF and addresses the CodeQL request-forgery rule.
// Safe: URL validated by security.ValidateExternalURL() which validates URL
// format/scheme and blocks private/reserved destinations through DNS+dial-time checks.
// Safe: URL validated by security.ValidateExternalURL() which:
// 1. Validates URL format and scheme (HTTPS required in production)
// 2. Resolves DNS and blocks private/reserved IPs (RFC 1918, loopback, link-local)
// 3. Uses ssrfSafeDialer for connection-time IP revalidation (TOCTOU protection)
// 4. No redirect following allowed
// See: internal/security/url_validator.go
resp, err := client.Do(req)
resp, err := webhookDoRequestFunc(client, req)
if err != nil {
return fmt.Errorf("failed to send webhook: %w", err)
}
@@ -459,6 +402,10 @@ func isValidRedirectURL(rawURL string) bool {
}
func (s *NotificationService) TestProvider(provider models.NotificationProvider) error {
if err := validateDiscordProviderURL(provider.Type, provider.URL); err != nil {
return err
}
if supportsJSONTemplates(provider.Type) && provider.Template != "" {
data := map[string]any{
"Title": "Test Notification",
@@ -574,6 +521,10 @@ func (s *NotificationService) ListProviders() ([]models.NotificationProvider, er
}
func (s *NotificationService) CreateProvider(provider *models.NotificationProvider) error {
if err := validateDiscordProviderURL(provider.Type, provider.URL); err != nil {
return err
}
// Validate custom template before creating
if strings.ToLower(strings.TrimSpace(provider.Template)) == "custom" && strings.TrimSpace(provider.Config) != "" {
// Provide a minimal preview payload
@@ -586,6 +537,10 @@ func (s *NotificationService) CreateProvider(provider *models.NotificationProvid
}
func (s *NotificationService) UpdateProvider(provider *models.NotificationProvider) error {
if err := validateDiscordProviderURL(provider.Type, provider.URL); err != nil {
return err
}
// Validate custom template before saving
if strings.ToLower(strings.TrimSpace(provider.Template)) == "custom" && strings.TrimSpace(provider.Config) != "" {
payload := map[string]any{"Title": "Preview", "Message": "Preview", "Time": time.Now().Format(time.RFC3339), "EventType": "preview"}

View File

@@ -43,6 +43,91 @@ func TestSupportsJSONTemplates(t *testing.T) {
}
}
func TestSendJSONPayload_DiscordIPHostRejected(t *testing.T) {
db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{})
require.NoError(t, err)
require.NoError(t, db.AutoMigrate(&models.NotificationProvider{}))
svc := NewNotificationService(db)
provider := models.NotificationProvider{
Type: "discord",
URL: "https://203.0.113.10/api/webhooks/123456/token_abc",
Template: "custom",
Config: `{"content": {{toJSON .Message}}, "username": "Charon"}`,
}
data := map[string]any{
"Message": "Test notification",
"Title": "Test",
"Time": time.Now().Format(time.RFC3339),
}
err = svc.sendJSONPayload(context.Background(), provider, data)
require.Error(t, err)
assert.Contains(t, err.Error(), "invalid Discord webhook URL")
assert.Contains(t, err.Error(), "IP address hosts are not allowed")
}
func TestValidateDiscordWebhookURL_AcceptsDiscordHostname(t *testing.T) {
err := validateDiscordWebhookURL("https://discord.com/api/webhooks/123456/token_abc?wait=true")
assert.NoError(t, err)
}
func TestValidateDiscordWebhookURL_AcceptsCanaryDiscordHostname(t *testing.T) {
err := validateDiscordWebhookURL("https://canary.discord.com/api/webhooks/123456/token_abc")
assert.NoError(t, err)
}
func TestValidateDiscordProviderURL_NonDiscordUnchanged(t *testing.T) {
err := validateDiscordProviderURL("webhook", "https://203.0.113.20/hooks/test?x=1#y")
assert.NoError(t, err)
}
func TestSendJSONPayload_UsesStoredHostnameURLWithoutHostMutation(t *testing.T) {
db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{})
require.NoError(t, err)
svc := NewNotificationService(db)
var observedURLHost string
var observedRequestHost string
originalDo := webhookDoRequestFunc
defer func() { webhookDoRequestFunc = originalDo }()
webhookDoRequestFunc = func(client *http.Client, req *http.Request) (*http.Response, error) {
observedURLHost = req.URL.Host
observedRequestHost = req.Host
return client.Do(req)
}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
defer server.Close()
parsedServerURL, err := url.Parse(server.URL)
require.NoError(t, err)
parsedServerURL.Host = "localhost:" + parsedServerURL.Port()
provider := models.NotificationProvider{
Type: "webhook",
URL: parsedServerURL.String(),
Template: "minimal",
}
data := map[string]any{
"Message": "Test notification",
"Title": "Test",
"Time": time.Now().Format(time.RFC3339),
}
err = svc.sendJSONPayload(context.Background(), provider, data)
require.NoError(t, err)
assert.Equal(t, "localhost:"+parsedServerURL.Port(), observedURLHost)
assert.Equal(t, observedURLHost, observedRequestHost)
}
func TestSendJSONPayload_Discord(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "POST", r.Method)
@@ -66,7 +151,7 @@ func TestSendJSONPayload_Discord(t *testing.T) {
svc := NewNotificationService(db)
provider := models.NotificationProvider{
Type: "discord",
Type: "webhook",
URL: server.URL,
Template: "custom",
Config: `{"content": {{toJSON .Message}}, "username": "Charon"}`,
@@ -82,31 +167,6 @@ func TestSendJSONPayload_Discord(t *testing.T) {
assert.NoError(t, err)
}
func TestCanonicalizeDiscordWebhookURL_DiscordIPHostToDiscordDomain(t *testing.T) {
raw := "https://203.0.113.10/api/webhooks/123456/token_abc?wait=true#frag"
got := canonicalizeDiscordWebhookURL("discord", raw)
parsed, err := url.Parse(got)
require.NoError(t, err)
assert.Equal(t, "https", parsed.Scheme)
assert.Equal(t, "discord.com", parsed.Hostname())
assert.Equal(t, "/api/webhooks/123456/token_abc", parsed.Path)
assert.Equal(t, "wait=true", parsed.RawQuery)
assert.Equal(t, "frag", parsed.Fragment)
}
func TestCanonicalizeDiscordWebhookURL_NonDiscordUnchanged(t *testing.T) {
raw := "https://203.0.113.20/hooks/test?x=1#y"
got := canonicalizeDiscordWebhookURL("webhook", raw)
assert.Equal(t, raw, got)
}
func TestCanonicalizeDiscordWebhookURL_DiscordNonIPUnchanged(t *testing.T) {
raw := "https://discord.com/api/webhooks/123456/token_abc?wait=true"
got := canonicalizeDiscordWebhookURL("discord", raw)
assert.Equal(t, raw, got)
}
func TestSendJSONPayload_Slack(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var payload map[string]any
@@ -232,15 +292,6 @@ func TestSendJSONPayload_TemplateSizeLimit(t *testing.T) {
}
func TestSendJSONPayload_DiscordValidation(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var payload map[string]any
err := json.NewDecoder(r.Body).Decode(&payload)
require.NoError(t, err)
assert.Equal(t, "Test", payload["content"])
w.WriteHeader(http.StatusOK)
}))
defer server.Close()
db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{})
require.NoError(t, err)
@@ -248,7 +299,7 @@ func TestSendJSONPayload_DiscordValidation(t *testing.T) {
provider := models.NotificationProvider{
Type: "discord",
URL: server.URL,
URL: "https://203.0.113.10/api/webhooks/123456/token_abc",
Template: "custom",
Config: `{"username": "Charon", "message": {{toJSON .Message}}}`,
}
@@ -258,7 +309,9 @@ func TestSendJSONPayload_DiscordValidation(t *testing.T) {
}
err = svc.sendJSONPayload(context.Background(), provider, data)
assert.NoError(t, err)
assert.Error(t, err)
assert.Contains(t, err.Error(), "invalid Discord webhook URL")
assert.Contains(t, err.Error(), "IP address hosts are not allowed")
}
func TestSendJSONPayload_DiscordValidation_MissingMessage(t *testing.T) {
@@ -269,7 +322,7 @@ func TestSendJSONPayload_DiscordValidation_MissingMessage(t *testing.T) {
provider := models.NotificationProvider{
Type: "discord",
URL: "http://localhost:9999",
URL: "https://discord.com/api/webhooks/123456/token_abc",
Template: "custom",
Config: `{"username": "Charon"}`,
}
@@ -401,7 +454,7 @@ func TestSendExternal_UsesJSONForSupportedServices(t *testing.T) {
defer server.Close()
provider := models.NotificationProvider{
Type: "discord",
Type: "webhook",
URL: server.URL,
Template: "custom",
Config: `{"content": {{toJSON .Message}}}`,
@@ -415,7 +468,7 @@ func TestSendExternal_UsesJSONForSupportedServices(t *testing.T) {
// Give goroutine time to execute
time.Sleep(100 * time.Millisecond)
assert.True(t, called.Load(), "Discord notification should have been sent via JSON")
assert.True(t, called.Load(), "notification should have been sent via JSON")
}
func TestTestProvider_UsesJSONForSupportedServices(t *testing.T) {
@@ -434,7 +487,7 @@ func TestTestProvider_UsesJSONForSupportedServices(t *testing.T) {
svc := NewNotificationService(db)
provider := models.NotificationProvider{
Type: "discord",
Type: "webhook",
URL: server.URL,
Template: "custom",
Config: `{"content": {{toJSON .Message}}}`,

View File

@@ -97,7 +97,7 @@ func TestNotificationService_Providers(t *testing.T) {
provider := models.NotificationProvider{
Name: "Discord",
Type: "discord",
URL: "http://example.com",
URL: "https://discord.com/api/webhooks/123456/token_abc",
}
err := svc.CreateProvider(&provider)
require.NoError(t, err)