feat(clientcache): add more test for sort options

internal/clientcache/internal/cache/options.go

@@ -6,13 +6,19 @@ package cache
 import (
 	stderrors "errors"
 	"fmt"
-	"strings"
+	"regexp"
+	"slices"
 
 	"github.com/hashicorp/go-dbw"
 )
 
-// unsafeSortChars contains characters that could break SQL ORDER BY clauses
-const unsafeSortChars = " \t\n\r,;()'\"\\-"
+// safeSortColumnRegex contains characters that could break SQL ORDER BY clauses
+var safeSortColumnRegex = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)
+
+var (
+	errInvalidSortColumn = stderrors.New("not allowed for this resource type")
+	errUnsafeSortColumn  = stderrors.New("contains unsafe characters")
+)
 
 type testRefreshWaitChs struct {
 	firstSempahore  chan struct{}
@@ -152,15 +158,16 @@ func WithUseNonPagedListing(b bool) Option {
 // Validates column against sortableColumns and rejects SQL-unsafe characters.
 func WithSort(sortBy SortBy, direction SortDirection, sortableColumns []SortBy) Option {
 	return func(o *options) error {
+		// ignore empty sortBy
 		if sortBy == SortByDefault {
 			return nil
 		}
 
 		switch {
-		case !sliceContains(sortableColumns, sortBy):
-			return fmt.Errorf("invalid sort column %q: not allowed for this resource type", sortBy)
-		case strings.ContainsAny(string(sortBy), unsafeSortChars):
-			return fmt.Errorf("invalid sort column %q: contains unsafe characters", sortBy)
+		case !slices.Contains(sortableColumns, sortBy):
+			return fmt.Errorf("invalid sort column %q: %w", sortBy, errInvalidSortColumn)
+		case !safeSortColumnRegex.MatchString(string(sortBy)):
+			return fmt.Errorf("invalid sort column %q: %w", sortBy, errUnsafeSortColumn)
 		}
 
 		o.withSortBy = sortBy
@@ -168,13 +175,3 @@ func WithSort(sortBy SortBy, direction SortDirection, sortableColumns []SortBy)
 		return nil
 	}
 }
-
-// sliceContains checks if a value exists in a slice.
-func sliceContains[T comparable](slice []T, val T) bool {
-	for _, v := range slice {
-		if v == val {
-			return true
-		}
-	}
-	return false
-}

internal/clientcache/internal/cache/options_test.go

@@ -157,27 +157,67 @@ func Test_GetOpts(t *testing.T) {
 	t.Run("WithSort-column-not-in-sortable-list", func(t *testing.T) {
 		_, err := getOpts(WithSort(SortByName, Ascending, []SortBy{SortByCreatedAt}))
 		require.Error(t, err)
-		assert.ErrorContains(t, err, "not allowed for this resource type")
+		assert.ErrorContains(t, err, errInvalidSortColumn.Error())
 	})
 	t.Run("WithSort-empty-sortable-columns", func(t *testing.T) {
 		_, err := getOpts(WithSort(SortByName, Ascending, []SortBy{}))
 		require.Error(t, err)
-		assert.ErrorContains(t, err, "not allowed for this resource type")
+		assert.ErrorContains(t, err, errInvalidSortColumn.Error())
 	})
 	t.Run("WithSort-nil-sortable-columns", func(t *testing.T) {
 		_, err := getOpts(WithSort(SortByName, Ascending, nil))
 		require.Error(t, err)
-		assert.ErrorContains(t, err, "not allowed for this resource type")
+		assert.ErrorContains(t, err, errInvalidSortColumn.Error())
 	})
 	t.Run("WithSort-unsafe-chars-semicolon", func(t *testing.T) {
 		_, err := getOpts(WithSort(SortBy("name; DROP TABLE"), Ascending, []SortBy{SortBy("name; DROP TABLE")}))
 		require.Error(t, err)
-		assert.ErrorContains(t, err, "contains unsafe characters")
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
 	})
 	t.Run("WithSort-unsafe-chars-quote", func(t *testing.T) {
 		_, err := getOpts(WithSort(SortBy("name'--"), Ascending, []SortBy{SortBy("name'--")}))
 		require.Error(t, err)
-		assert.ErrorContains(t, err, "contains unsafe characters")
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-double-quote", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name\"--"), Ascending, []SortBy{SortBy("name\"--")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-backslash", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name\\x00"), Ascending, []SortBy{SortBy("name\\x00")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-comma", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name,other"), Ascending, []SortBy{SortBy("name,other")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-parenthesis", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name("), Ascending, []SortBy{SortBy("name(")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-space", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name "), Ascending, []SortBy{SortBy("name ")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-tab", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name\t"), Ascending, []SortBy{SortBy("name\t")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-newline", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name\n"), Ascending, []SortBy{SortBy("name\n")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
+	})
+	t.Run("WithSort-unsafe-chars-dash", func(t *testing.T) {
+		_, err := getOpts(WithSort(SortBy("name-col"), Ascending, []SortBy{SortBy("name-col")}))
+		require.Error(t, err)
+		assert.ErrorContains(t, err, errUnsafeSortColumn.Error())
 	})
 	t.Run("WithSort-default-direction", func(t *testing.T) {
 		opts, err := getOpts(WithSort(SortByName, SortDirectionDefault, []SortBy{SortByName}))