diff --git a/agent/cmd/chat.go b/agent/cmd/chat.go index de8336a3e..a21852e20 100644 --- a/agent/cmd/chat.go +++ b/agent/cmd/chat.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "math" "net/http" "regexp" "strings" @@ -123,6 +124,11 @@ func (r chatRequest) validate(action string) error { if action == "timeout" && (r.Length == 0 || r.Reason == "") { return invalid("missing provider, channelId, username, length, reason") } + // Twitch's documented timeout bounds; checked here so that a 400 from + // Twitch can only be a refusal of the moderation action. + if action == "timeout" && r.Provider == "twitch" && (r.Length < 1 || r.Length > 1209600 || r.Length != math.Trunc(r.Length) || utf8.RuneCountInString(r.Reason) > 500) { + return invalid("invalid length, reason") + } case "deleteMessage": if r.ChannelID == "" || r.MessageID == "" { return invalid("missing provider, channelId, messageId") diff --git a/agent/cmd/chat_moderation_test.go b/agent/cmd/chat_moderation_test.go index 1db735783..ff2641ec1 100644 --- a/agent/cmd/chat_moderation_test.go +++ b/agent/cmd/chat_moderation_test.go @@ -29,8 +29,13 @@ func TestTwitchModerationRejectionReporting(t *testing.T) { {"unban forbidden", "unban", "private-provider-detail", 403, 500, "INTERNAL", "INTERNAL"}, {"documented wording", "ban", "The user specified in the user_id field is already banned.", 400, 400, "FAILED_PRECONDITION", "This user is already banned."}, {"missing provider parameter", "ban", "user_id is required", 400, 500, "INTERNAL", "INTERNAL"}, - {"unexpected response", "ban", "private-provider-detail", 400, 500, "INTERNAL", "INTERNAL"}, - {"not a matching reason", "ban", "user is already banned private-provider-detail", 400, 500, "INTERNAL", "INTERNAL"}, + {"unrecognized ban rejection", "ban", "private-provider-detail", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this ban request."}, + {"unrecognized timeout rejection", "timeout", "private-provider-detail", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this timeout request."}, + {"not a matching reason", "ban", "user is already banned private-provider-detail", 400, 400, "FAILED_PRECONDITION", "Twitch rejected this ban request."}, + {"empty ban rejection", "ban", "", 400, 500, "INTERNAL", "INTERNAL"}, + {"missing query parameter", "ban", `Missing required parameter "moderator_id"`, 400, 500, "INTERNAL", "INTERNAL"}, + {"malformed parameter", "timeout", "The value in the duration field is not valid.", 400, 500, "INTERNAL", "INTERNAL"}, + {"malformed unban", "unban", "The user_id query parameter is required.", 400, 500, "INTERNAL", "INTERNAL"}, {"forbidden", "ban", "private-provider-detail", 403, 500, "INTERNAL", "INTERNAL"}, {"server failure with known text", "ban", "user is already banned", 503, 500, "INTERNAL", "INTERNAL"}, {"rate limited", "ban", "private-provider-detail", 429, 429, "RESOURCE_EXHAUSTED", "too many requests, try again shortly"}, @@ -116,3 +121,27 @@ func TestTwitchUnbanMalformedRejection(t *testing.T) { }) } } + +func TestTwitchTimeoutBoundsCheckedLocally(t *testing.T) { + for _, tc := range []struct { + name string + length float64 + reason string + }{ + {"too long", 1209601, "moderation"}, + {"fractional", 1.5, "moderation"}, + {"negative", -60, "moderation"}, + {"long reason", 60, strings.Repeat("é", 501)}, + } { + t.Run(tc.name, func(t *testing.T) { + s, _ := newMock(func(r *http.Request) (*http.Response, error) { + t.Fatalf("invalid timeout sent to Twitch: %s", r.URL.Path) + return nil, nil + }) + code, body := call(t, s, "timeout", map[string]any{"provider": "twitch", "channelId": "broadcaster", "userId": "target", "length": tc.length, "reason": tc.reason}, true) + if code != 400 || object(body["error"])["status"] != "INVALID_ARGUMENT" { + t.Fatal(code, body) + } + }) + } +} diff --git a/agent/internal/clients/twitch/chat.go b/agent/internal/clients/twitch/chat.go index 0f8bbd073..a01c56432 100644 --- a/agent/internal/clients/twitch/chat.go +++ b/agent/internal/clients/twitch/chat.go @@ -3,6 +3,7 @@ package twitch import ( "context" "net/url" + "regexp" "strings" "time" ) @@ -67,11 +68,15 @@ func (c *Client) Ban(ctx context.Context, channel, moderator, user string, durat if duration != 0 { data["duration"], data["reason"] = duration, reason } - return c.writeBan(ctx, "POST", moderationPath("/moderation/bans", channel, moderator, nil), map[string]any{"data": data}) + action := "ban" + if duration != 0 { + action = "timeout" + } + return c.writeBan(ctx, action, "POST", moderationPath("/moderation/bans", channel, moderator, nil), map[string]any{"data": data}) } func (c *Client) Unban(ctx context.Context, channel, moderator, user string) error { - return c.writeBan(ctx, "DELETE", moderationPath("/moderation/bans", channel, moderator, url.Values{"user_id": {user}}), nil) + return c.writeBan(ctx, "unban", "DELETE", moderationPath("/moderation/bans", channel, moderator, url.Values{"user_id": {user}}), nil) } // ModerationRejected is a refusal of a moderation action by Twitch. @@ -80,7 +85,11 @@ type ModerationRejected struct{ Message string } func (e *ModerationRejected) Error() string { return e.Message } -func (c *Client) writeBan(ctx context.Context, method, path string, body any) error { +// Helix describes a malformed request in terms of its parameters, while a +// refusal describes the target user. Malformed requests are agent bugs. +var malformedRequest = regexp.MustCompile(`parameter|required|malformed|valid`) + +func (c *Client) writeBan(ctx context.Context, action, method, path string, body any) error { response, err := c.Do(ctx, method, path, body) if err != nil { return err @@ -99,10 +108,12 @@ func (c *Client) writeBan(ctx context.Context, method, path string, body any) er case "user may not be put in a timeout", "the user specified in the user_id field may not be put in a timeout": return &ModerationRejected{Message: "Twitch does not allow timing out this user."} } - // Unban has no configurable body. A valid rejection should reach the - // caller even when Twitch changes its wording. Never expose raw text. - if method == "DELETE" && strings.TrimSpace(result.Message) != "" { - return &ModerationRejected{Message: "Twitch rejected this unban request."} + // The request carries only resolved IDs and locally validated timeout + // fields, so any other rejection is Twitch refusing the action, even + // when it changes its wording. Never expose raw text. + message := strings.ToLower(strings.TrimSpace(result.Message)) + if message != "" && !malformedRequest.MatchString(message) { + return &ModerationRejected{Message: "Twitch rejected this " + action + " request."} } } }