From ed0d359c2faa43375160ae39b29e6e8ac126dddb Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Wed, 10 Jun 2026 23:30:13 +0530 Subject: [PATCH] feat: cap total filter conditions and 'in' values in query builder --- internal/conversation/conversation.go | 50 +++++++++++++-------------- internal/dbutil/builder.go | 26 ++++++++++++++ internal/dbutil/builder_test.go | 21 +++++++++++ 3 files changed, 72 insertions(+), 25 deletions(-) diff --git a/internal/conversation/conversation.go b/internal/conversation/conversation.go index 932d3f88..6a5a9202 100644 --- a/internal/conversation/conversation.go +++ b/internal/conversation/conversation.go @@ -1643,31 +1643,6 @@ func (c *Manager) ValidateListFilters(filtersJSON string) error { return envelope.NewError(envelope.InputError, c.i18n.T("globals.messages.invalidFilters"), nil) } -func renderTagFilter(operator, value string, paramIndex int) (string, []any, error) { - switch operator { - case "contains", "not contains": - var tagIDs []int - if err := json.Unmarshal([]byte(value), &tagIDs); err != nil { - return "", nil, fmt.Errorf("invalid tag IDs in filter: %w", err) - } - if len(tagIDs) == 0 { - return "", nil, nil - } - op := "IN" - if operator == "not contains" { - op = "NOT IN" - } - sql := fmt.Sprintf("conversations.id %s (SELECT DISTINCT conversation_id FROM conversation_tags WHERE tag_id = ANY($%d::int[]))", op, paramIndex) - return sql, []any{pq.Array(tagIDs)}, nil - case "set": - return "EXISTS (SELECT 1 FROM conversation_tags WHERE conversation_id = conversations.id)", nil, nil - case "not set": - return "NOT EXISTS (SELECT 1 FROM conversation_tags WHERE conversation_id = conversations.id)", nil, nil - default: - return "", nil, fmt.Errorf("invalid operator for tags: %s", operator) - } -} - // ProcessCSATStatus processes messages and adds CSAT submission status for CSAT messages. func (m *Manager) ProcessCSATStatus(messages []models.Message) { for i := range messages { @@ -1949,3 +1924,28 @@ func nullTimeOrNil(t null.Time) any { } return t.Time.Format(time.RFC3339) } + +func renderTagFilter(operator, value string, paramIndex int) (string, []any, error) { + switch operator { + case "contains", "not contains": + var tagIDs []int + if err := json.Unmarshal([]byte(value), &tagIDs); err != nil { + return "", nil, fmt.Errorf("invalid tag IDs in filter: %w", err) + } + if len(tagIDs) == 0 { + return "", nil, nil + } + op := "IN" + if operator == "not contains" { + op = "NOT IN" + } + sql := fmt.Sprintf("conversations.id %s (SELECT DISTINCT conversation_id FROM conversation_tags WHERE tag_id = ANY($%d::int[]))", op, paramIndex) + return sql, []any{pq.Array(tagIDs)}, nil + case "set": + return "EXISTS (SELECT 1 FROM conversation_tags WHERE conversation_id = conversations.id)", nil, nil + case "not set": + return "NOT EXISTS (SELECT 1 FROM conversation_tags WHERE conversation_id = conversations.id)", nil, nil + default: + return "", nil, fmt.Errorf("invalid operator for tags: %s", operator) + } +} diff --git a/internal/dbutil/builder.go b/internal/dbutil/builder.go index 53eaac21..4844496f 100644 --- a/internal/dbutil/builder.go +++ b/internal/dbutil/builder.go @@ -32,6 +32,12 @@ const maxFilterDepth = 2 // MaxFilterGroups bounds how many groups a filter may contain (excluding the root). const MaxFilterGroups = 10 +// maxFilterConditions bounds how many leaf conditions a filter may contain in total. +const maxFilterConditions = 50 + +// maxInValues bounds how many values an "in" condition may carry. +const maxInValues = 100 + // PaginationOptions represents the options for paginating a query. type PaginationOptions struct { Page int @@ -196,6 +202,20 @@ func countGroups(node FilterNode) int { return n } +func countConditions(node FilterNode) int { + if !node.isGroup() { + if node.isEmpty() { + return 0 + } + return 1 + } + n := 0 + for _, child := range node.Rules { + n += countConditions(child) + } + return n +} + func buildNode(node FilterNode, args *[]any, next *int, allowedFields AllowedFields, renderers FieldRenderers, depth int) (string, error) { if depth > maxFilterDepth { return "", fmt.Errorf("filter nesting too deep") @@ -209,6 +229,9 @@ func buildNode(node FilterNode, args *[]any, next *int, allowedFields AllowedFie if groups > MaxFilterGroups { return "", ErrTooManyGroups } + if countConditions(node) > maxFilterConditions { + return "", fmt.Errorf("filter has too many conditions (max %d)", maxFilterConditions) + } } if node.isEmpty() { @@ -329,6 +352,9 @@ func buildLeaf(f FilterNode, args *[]any, next *int, allowedFields AllowedFields if len(arr) == 0 { return "", fmt.Errorf("operator \"in\" requires at least one value") } + if len(arr) > maxInValues { + return "", fmt.Errorf("operator \"in\" allows at most %d values", maxInValues) + } placeholders := make([]string, len(arr)) for i, v := range arr { placeholders[i] = fmt.Sprintf("$%d", *next) diff --git a/internal/dbutil/builder_test.go b/internal/dbutil/builder_test.go index 1bab6e41..60ecc63b 100644 --- a/internal/dbutil/builder_test.go +++ b/internal/dbutil/builder_test.go @@ -140,6 +140,27 @@ func TestTooManyGroupsRejected(t *testing.T) { } } +func TestTooManyConditionsRejected(t *testing.T) { + leaf := `{"model":"conversations","field":"status_id","operator":"equals","value":"1"}` + leaves := make([]string, maxFilterConditions+1) + for i := range leaves { + leaves[i] = leaf + } + if _, _, err := build(t, `[`+strings.Join(leaves, ",")+`]`); err == nil { + t.Fatalf("expected error for more than %d conditions", maxFilterConditions) + } +} + +func TestTooManyInValuesRejected(t *testing.T) { + vals := make([]string, maxInValues+1) + for i := range vals { + vals[i] = `"1"` + } + if _, _, err := build(t, `[{"model":"conversations","field":"status_id","operator":"in","value":"[`+strings.ReplaceAll(strings.Join(vals, ","), `"`, `\"`)+`]"}]`); err == nil { + t.Fatal("expected error for oversized 'in' array") + } +} + func TestEmptyInRejected(t *testing.T) { if _, _, err := build(t, `[{"model":"conversations","field":"status_id","operator":"in","value":"[]"}]`); err == nil { t.Fatal("expected error for empty 'in' array")