From d00684baa81c586f82bdfdc22636bba5f40948b7 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Thu, 15 Jan 2026 13:10:25 -0800 Subject: [PATCH] feat(clientcache): add more test for sort options --- .../clientcache/internal/cache/options.go | 31 ++++++------ .../internal/cache/options_test.go | 50 +++++++++++++++++-- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/internal/clientcache/internal/cache/options.go b/internal/clientcache/internal/cache/options.go index 1a32c61519..f8801b4268 100644 --- a/internal/clientcache/internal/cache/options.go +++ b/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 -} diff --git a/internal/clientcache/internal/cache/options_test.go b/internal/clientcache/internal/cache/options_test.go index c8aa355a4e..8a513436f2 100644 --- a/internal/clientcache/internal/cache/options_test.go +++ b/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}))