Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ListHistoryEvaluation filtering fixes. #3866

Merged
merged 1 commit into from
Jul 11, 2024
Merged

ListHistoryEvaluation filtering fixes. #3866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions database/query/eval_history.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ SELECT s.id::uuid AS evaluation_id,
AND (sqlc.narg(prev)::timestamp without time zone IS NULL OR sqlc.narg(prev) < s.most_recent_evaluation)
-- inclusion filters
AND (sqlc.slice(entityTypes)::entities[] IS NULL OR entity_type::entities = ANY(sqlc.slice(entityTypes)::entities[]))
AND (sqlc.slice(entityNames)::text[] IS NULL OR ere.repository_id IS NULL OR r.repo_name = ANY(sqlc.slice(entityNames)::text[]))
AND (sqlc.slice(entityNames)::text[] IS NULL OR ere.repository_id IS NULL OR CONCAT(r.repo_owner, '/', r.repo_name) = ANY(sqlc.slice(entityNames)::text[]))
AND (sqlc.slice(entityNames)::text[] IS NULL OR ere.pull_request_id IS NULL OR pr.pr_number::text = ANY(sqlc.slice(entityNames)::text[]))
AND (sqlc.slice(entityNames)::text[] IS NULL OR ere.artifact_id IS NULL OR a.artifact_name = ANY(sqlc.slice(entityNames)::text[]))
AND (sqlc.slice(profileNames)::text[] IS NULL OR p.name = ANY(sqlc.slice(profileNames)::text[]))
Expand All @@ -150,7 +150,7 @@ SELECT s.id::uuid AS evaluation_id,
AND (sqlc.slice(statuses)::eval_status_types[] IS NULL OR s.status = ANY(sqlc.slice(statuses)::eval_status_types[]))
-- exclusion filters
AND (sqlc.slice(notEntityTypes)::entities[] IS NULL OR entity_type::entities != ANY(sqlc.slice(notEntityTypes)::entities[]))
AND (sqlc.slice(notEntityNames)::text[] IS NULL OR ere.repository_id IS NULL OR r.repo_name != ANY(sqlc.slice(notEntityNames)::text[]))
AND (sqlc.slice(notEntityNames)::text[] IS NULL OR ere.repository_id IS NULL OR CONCAT(r.repo_owner, '/', r.repo_name) != ANY(sqlc.slice(notEntityNames)::text[]))
AND (sqlc.slice(notEntityNames)::text[] IS NULL OR ere.pull_request_id IS NULL OR pr.pr_number::text != ANY(sqlc.slice(notEntityNames)::text[]))
AND (sqlc.slice(notEntityNames)::text[] IS NULL OR ere.artifact_id IS NULL OR a.artifact_name != ANY(sqlc.slice(notEntityNames)::text[]))
AND (sqlc.slice(notProfileNames)::text[] IS NULL OR p.name != ANY(sqlc.slice(notProfileNames)::text[]))
Expand Down
4 changes: 2 additions & 2 deletions internal/db/eval_history.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 20 additions & 4 deletions internal/history/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/base64"
"errors"
"fmt"
"slices"
"strconv"
"strings"
"time"
Expand All @@ -44,6 +45,13 @@ var (
ErrInvalidIdentifier = errors.New("invalid identifier")
)

var (
allowedEntityTypes = []string{"repository", "build_environment", "artifact", "pull_request"}
allowedEvaluationStatuses = []string{"success", "failure", "error", "skipped", "pending"}
allowedRemediationStatuses = []string{"success", "failure", "error", "skipped", "not_available", "pending"}
allowedAlertStatuses = []string{"on", "off", "error", "skipped", "not_available"}
)

// Direction enumerates the direction of the Cursor.
type Direction string

Expand Down Expand Up @@ -294,6 +302,9 @@ func (filter *listEvaluationFilter) AddEntityType(entityType string) error {
} else {
filter.includedEntityTypes = append(filter.includedEntityTypes, entityType)
}
if !slices.Contains(allowedEntityTypes, entityType) {
return fmt.Errorf("%w: entity type", ErrInvalidIdentifier)
}

// Prevent filtering for both inclusion and exclusion
if len(filter.includedEntityTypes) > 0 &&
Expand Down Expand Up @@ -363,6 +374,9 @@ func (filter *listEvaluationFilter) AddStatus(status string) error {
} else {
filter.includedStatuses = append(filter.includedStatuses, status)
}
if !slices.Contains(allowedEvaluationStatuses, status) {
return fmt.Errorf("%w: status", ErrInvalidIdentifier)
}

// Prevent filtering for both inclusion and exclusion
if len(filter.includedStatuses) > 0 &&
Expand All @@ -386,6 +400,9 @@ func (filter *listEvaluationFilter) AddRemediation(remediation string) error {
} else {
filter.includedRemediations = append(filter.includedRemediations, remediation)
}
if !slices.Contains(allowedRemediationStatuses, remediation) {
return fmt.Errorf("%w: remediation", ErrInvalidIdentifier)
}

// Prevent filtering for both inclusion and exclusion
if len(filter.includedRemediations) > 0 &&
Expand All @@ -409,6 +426,9 @@ func (filter *listEvaluationFilter) AddAlert(alert string) error {
} else {
filter.includedAlerts = append(filter.includedAlerts, alert)
}
if !slices.Contains(allowedAlertStatuses, alert) {
return fmt.Errorf("%w: alert", ErrInvalidIdentifier)
}

// Prevent filtering for both inclusion and exclusion
if len(filter.includedAlerts) > 0 &&
Expand Down Expand Up @@ -483,7 +503,6 @@ func WithEntityType(entityType string) FilterOpt {
if !ok {
return fmt.Errorf("%w: wrong filter type", ErrInvalidIdentifier)
}
// TODO add validation on enumerated types
return inner.AddEntityType(entityType)
}
}
Expand Down Expand Up @@ -532,7 +551,6 @@ func WithStatus(status string) FilterOpt {
if !ok {
return fmt.Errorf("%w: wrong filter type", ErrInvalidIdentifier)
}
// TODO add validation on enumerated types
return inner.AddStatus(status)
}
}
Expand All @@ -549,7 +567,6 @@ func WithRemediation(remediation string) FilterOpt {
if !ok {
return fmt.Errorf("%w: wrong filter type", ErrInvalidIdentifier)
}
// TODO add validation on enumerated types
return inner.AddRemediation(remediation)
}
}
Expand All @@ -566,7 +583,6 @@ func WithAlert(alert string) FilterOpt {
if !ok {
return fmt.Errorf("%w: wrong filter type", ErrInvalidIdentifier)
}
// TODO add validation on enumerated types
return inner.AddAlert(alert)
}
}
Expand Down
36 changes: 18 additions & 18 deletions internal/history/models_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,8 @@ func TestListEvaluationFilter(t *testing.T) {
filter: func(t *testing.T) (ListEvaluationFilter, error) {
t.Helper()
return NewListEvaluationFilter(
WithAlert("success"),
WithAlert("!failure"),
WithAlert("on"),
WithAlert("!off"),
)
},
err: true,
Expand Down Expand Up @@ -491,7 +491,7 @@ func TestFilterOptions(t *testing.T) {
name: "bogus entity type",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithEntityType("!")
return WithEntityType("foo")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand Down Expand Up @@ -665,7 +665,7 @@ func TestFilterOptions(t *testing.T) {
name: "status in filter",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithStatus("repository")
return WithStatus("success")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -675,15 +675,15 @@ func TestFilterOptions(t *testing.T) {
t.Helper()
f := filter.(StatusFilter)
require.NotNil(t, f.IncludedStatuses())
require.Equal(t, []string{"repository"}, f.IncludedStatuses())
require.Equal(t, []string{"success"}, f.IncludedStatuses())
require.Nil(t, f.ExcludedStatuses())
},
},
{
name: "status not in filter",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithStatus("!repository")
return WithStatus("!success")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -694,7 +694,7 @@ func TestFilterOptions(t *testing.T) {
f := filter.(StatusFilter)
require.Nil(t, f.IncludedStatuses())
require.NotNil(t, f.ExcludedStatuses())
require.Equal(t, []string{"repository"}, f.ExcludedStatuses())
require.Equal(t, []string{"success"}, f.ExcludedStatuses())
},
},
{
Expand All @@ -713,7 +713,7 @@ func TestFilterOptions(t *testing.T) {
name: "bogus status",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithStatus("!")
return WithStatus("foo")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -739,7 +739,7 @@ func TestFilterOptions(t *testing.T) {
name: "remediation in filter",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithRemediation("repository")
return WithRemediation("success")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -749,15 +749,15 @@ func TestFilterOptions(t *testing.T) {
t.Helper()
f := filter.(RemediationFilter)
require.NotNil(t, f.IncludedRemediations())
require.Equal(t, []string{"repository"}, f.IncludedRemediations())
require.Equal(t, []string{"success"}, f.IncludedRemediations())
require.Nil(t, f.ExcludedRemediations())
},
},
{
name: "remediation not in filter",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithRemediation("!repository")
return WithRemediation("!success")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -768,7 +768,7 @@ func TestFilterOptions(t *testing.T) {
f := filter.(RemediationFilter)
require.Nil(t, f.IncludedRemediations())
require.NotNil(t, f.ExcludedRemediations())
require.Equal(t, []string{"repository"}, f.ExcludedRemediations())
require.Equal(t, []string{"success"}, f.ExcludedRemediations())
},
},
{
Expand All @@ -787,7 +787,7 @@ func TestFilterOptions(t *testing.T) {
name: "bogus remediation",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithRemediation("!")
return WithRemediation("foo")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -813,7 +813,7 @@ func TestFilterOptions(t *testing.T) {
name: "alert in filter",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithAlert("repository")
return WithAlert("on")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -823,15 +823,15 @@ func TestFilterOptions(t *testing.T) {
t.Helper()
f := filter.(AlertFilter)
require.NotNil(t, f.IncludedAlerts())
require.Equal(t, []string{"repository"}, f.IncludedAlerts())
require.Equal(t, []string{"on"}, f.IncludedAlerts())
require.Nil(t, f.ExcludedAlerts())
},
},
{
name: "alert not in filter",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithAlert("!repository")
return WithAlert("!on")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand All @@ -842,7 +842,7 @@ func TestFilterOptions(t *testing.T) {
f := filter.(AlertFilter)
require.Nil(t, f.IncludedAlerts())
require.NotNil(t, f.ExcludedAlerts())
require.Equal(t, []string{"repository"}, f.ExcludedAlerts())
require.Equal(t, []string{"on"}, f.ExcludedAlerts())
},
},
{
Expand All @@ -861,7 +861,7 @@ func TestFilterOptions(t *testing.T) {
name: "bogus alert",
option: func(t *testing.T) FilterOpt {
t.Helper()
return WithAlert("!")
return WithAlert("foo")
},
filter: func(t *testing.T) Filter {
t.Helper()
Expand Down