diff --git a/workspace-server/internal/channels/discord.go b/workspace-server/internal/channels/discord.go index 453007d9e..d4eb1d0c5 100644 --- a/workspace-server/internal/channels/discord.go +++ b/workspace-server/internal/channels/discord.go @@ -33,7 +33,12 @@ const ( // // StartPolling returns nil immediately — Discord does not support long-polling; // use the Interactions webhook route instead. -type DiscordAdapter struct{} +type DiscordAdapter struct { + // httpClient overrides the default HTTP client used by SendMessage. + // Used in tests to inject a fake transport so token-redaction checks + // are deterministic and do not depend on real network behavior. + httpClient *http.Client +} func (d *DiscordAdapter) Type() string { return "discord" } func (d *DiscordAdapter) DisplayName() string { return "Discord" } @@ -95,7 +100,10 @@ func (d *DiscordAdapter) SendMessage(ctx context.Context, config map[string]inte // Split long messages into chunks at word boundaries where possible. chunks := splitMessage(text, maxLen) - client := &http.Client{Timeout: discordHTTPTimeout} + client := d.httpClient + if client == nil { + client = &http.Client{Timeout: discordHTTPTimeout} + } for _, chunk := range chunks { payload, err := json.Marshal(map[string]string{"content": chunk}) if err != nil { diff --git a/workspace-server/internal/channels/discord_test.go b/workspace-server/internal/channels/discord_test.go index d2b79ff39..61c89f4df 100644 --- a/workspace-server/internal/channels/discord_test.go +++ b/workspace-server/internal/channels/discord_test.go @@ -3,6 +3,7 @@ package channels import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" @@ -288,17 +289,25 @@ func TestSplitMessage_LongMessage(t *testing.T) { } // TestDiscordAdapter_SendMessage_ErrorDoesNotLeakToken verifies that when the -// HTTP call to the Discord webhook fails (e.g. DNS error), the returned error -// message does NOT contain the webhook URL — which embeds the Discord token. +// HTTP call to the Discord webhook fails, the returned error message does NOT +// contain the webhook URL — which embeds the Discord token. Uses a fake +// http.RoundTripper so the test is deterministic and does not depend on real +// network behavior (fixes RCA #1763 Finding 2). // Regression test for the MEDIUM security finding in PR #659. func TestDiscordAdapter_SendMessage_ErrorDoesNotLeakToken(t *testing.T) { - a := &DiscordAdapter{} - // Use a valid-looking webhook URL with a fake token so we can check it - // doesn't appear in the error string. fakeToken := "SUPER_SECRET_DISCORD_TOKEN_12345" webhookURL := discordWebhookPrefix + "123456789/" + fakeToken - // Point at an unroutable address to force a dial error. + // Fake transport that returns an error containing the full webhook URL, + // mimicking the real behavior of net/http when a dial fails. + fakeTransport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { + return nil, fmt.Errorf("dial tcp: connect to %s: connection refused", req.URL.String()) + }) + + a := &DiscordAdapter{ + httpClient: &http.Client{Transport: fakeTransport}, + } + err := a.SendMessage( context.Background(), map[string]interface{}{"webhook_url": webhookURL}, @@ -307,14 +316,20 @@ func TestDiscordAdapter_SendMessage_ErrorDoesNotLeakToken(t *testing.T) { ) if err == nil { - // In some environments the request might actually succeed; that's fine. - t.Skip("request unexpectedly succeeded — skipping token-leak check") + t.Fatal("expected error from fake transport, got nil") } if strings.Contains(err.Error(), fakeToken) { t.Errorf("error message leaks Discord webhook token: %q", err.Error()) } } +// roundTripperFunc adapts a function to the http.RoundTripper interface. +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + func TestSplitMessage_SplitsAtNewline(t *testing.T) { // Build a message where a newline falls within the split window. line1 := strings.Repeat("a", 1500) + "\n"