diff --git a/changes/issue-5624-packs-interval b/changes/issue-5624-packs-interval new file mode 100644 index 0000000000..91dd420af3 --- /dev/null +++ b/changes/issue-5624-packs-interval @@ -0,0 +1 @@ +* Added validation for pack scheduled query interval \ No newline at end of file diff --git a/cmd/fleetctl/apply_test.go b/cmd/fleetctl/apply_test.go index 69ce75b7d2..2349af9eb1 100644 --- a/cmd/fleetctl/apply_test.go +++ b/cmd/fleetctl/apply_test.go @@ -420,6 +420,26 @@ spec: require.Len(t, appliedPacks, 1) assert.Equal(t, "osquery_monitoring", appliedPacks[0].Name) require.Len(t, appliedPacks[0].Queries, 2) + + interval := writeTmpYml(t, `--- +apiVersion: v1 +kind: pack +spec: + name: test_bad_interval + queries: + - query: good_interval + name: good_interval + interval: 7200 + - query: bad_interval + name: bad_interval + interval: 604801 +`) + + expectedErrMsg := "applying packs: POST /api/latest/fleet/spec/packs received status 400 Bad request: pack payload verification: pack scheduled query interval must be an integer greater than 1 and less than 604800" + + _, err := runAppNoChecks([]string{"apply", "-f", interval}) + assert.Error(t, err) + require.Equal(t, expectedErrMsg, err.Error()) } func TestApplyQueries(t *testing.T) { diff --git a/frontend/pages/packs/EditPackPage/components/PackQueryEditorModal/PackQueryEditorModal.tsx b/frontend/pages/packs/EditPackPage/components/PackQueryEditorModal/PackQueryEditorModal.tsx index fff7c103ad..625423048c 100644 --- a/frontend/pages/packs/EditPackPage/components/PackQueryEditorModal/PackQueryEditorModal.tsx +++ b/frontend/pages/packs/EditPackPage/components/PackQueryEditorModal/PackQueryEditorModal.tsx @@ -13,6 +13,7 @@ import { IScheduledQuery } from "interfaces/scheduled_query"; import { PLATFORM_DROPDOWN_OPTIONS, LOGGING_TYPE_OPTIONS, + MAX_OSQUERY_SCHEDULED_QUERY_INTERVAL, MIN_OSQUERY_VERSION_OPTIONS, } from "utilities/constants"; @@ -69,6 +70,7 @@ const PackQueryEditorModal = ({ const [selectedFrequency, setSelectedFrequency] = useState( editQuery?.interval.toString() || "" ); + const [errorFrequency, setErrorFrequency] = useState(""); const [ selectedPlatformOptions, setSelectedPlatformOptions, @@ -108,6 +110,9 @@ const PackQueryEditorModal = ({ }; const onChangeFrequency = (value: string) => { + if (errorFrequency) { + setErrorFrequency(""); + } setSelectedFrequency(value); }; @@ -140,6 +145,7 @@ const PackQueryEditorModal = ({ }; const onFormSubmit = (): void => { + setErrorFrequency(""); const query_id = () => { if (editQuery) { return editQuery.query_id; @@ -147,6 +153,18 @@ const PackQueryEditorModal = ({ return selectedQuery?.id; }; + const frequency = parseInt(selectedFrequency, 10); + if (!frequency || frequency < 0) { + setErrorFrequency("Frequency must be an integer greater than zero"); + return; + } + if (frequency > MAX_OSQUERY_SCHEDULED_QUERY_INTERVAL) { + setErrorFrequency( + "Frequency must be an integer that does not exceed 604,800 (i.e. 7 days)" + ); + return; + } + onPackQueryFormSubmit( { interval: parseInt(selectedFrequency, 10), @@ -182,6 +200,7 @@ const PackQueryEditorModal = ({ )} MaxScheduledQueryInterval { + return errPackInvalidInterval + } + } return nil } diff --git a/server/service/global_schedule_test.go b/server/service/global_schedule_test.go index 0241213893..a9b5abf2c6 100644 --- a/server/service/global_schedule_test.go +++ b/server/service/global_schedule_test.go @@ -83,7 +83,7 @@ func TestGlobalScheduleAuth(t *testing.T) { _, err := svc.GetGlobalScheduledQueries(ctx, fleet.ListOptions{}) checkAuthErr(t, tt.shouldFailRead, err) - _, err = svc.GlobalScheduleQuery(ctx, &fleet.ScheduledQuery{Name: "query", QueryName: "query"}) + _, err = svc.GlobalScheduleQuery(ctx, &fleet.ScheduledQuery{Name: "query", QueryName: "query", Interval: 10}) checkAuthErr(t, tt.shouldFailWrite, err) _, err = svc.ModifyGlobalScheduledQueries(ctx, 1, fleet.ScheduledQueryPayload{}) diff --git a/server/service/scheduled_queries.go b/server/service/scheduled_queries.go index ca93e1b2be..74e3187d2f 100644 --- a/server/service/scheduled_queries.go +++ b/server/service/scheduled_queries.go @@ -107,6 +107,12 @@ func (svc *Service) ScheduleQuery(ctx context.Context, sq *fleet.ScheduledQuery) } func (svc *Service) unauthorizedScheduleQuery(ctx context.Context, sq *fleet.ScheduledQuery) (*fleet.ScheduledQuery, error) { + if sq.Interval < 1 || sq.Interval > fleet.MaxScheduledQueryInterval { + return nil, ctxerr.Wrap(ctx, &badRequestError{ + message: "invalid scheduled query interval", + }) + } + // Fill in the name with query name if it is unset (because the UI // doesn't provide a way to set it) if sq.Name == "" { @@ -129,6 +135,7 @@ func (svc *Service) unauthorizedScheduleQuery(ctx context.Context, sq *fleet.Sch } sq.QueryName = query.Name } + return svc.ds.NewScheduledQuery(ctx, sq) } @@ -222,6 +229,12 @@ func (svc *Service) ModifyScheduledQuery(ctx context.Context, id uint, p fleet.S } func (svc *Service) unauthorizedModifyScheduledQuery(ctx context.Context, id uint, p fleet.ScheduledQueryPayload) (*fleet.ScheduledQuery, error) { + if p.Interval != nil { + if *p.Interval < 1 || *p.Interval > fleet.MaxScheduledQueryInterval { + return nil, ctxerr.New(ctx, "invalid scheduled query interval") + } + } + sq, err := svc.ds.ScheduledQuery(ctx, id) if err != nil { return nil, ctxerr.Wrap(ctx, err, "getting scheduled query to modify") diff --git a/server/service/scheduled_queries_test.go b/server/service/scheduled_queries_test.go index 28153ce6dd..2169019552 100644 --- a/server/service/scheduled_queries_test.go +++ b/server/service/scheduled_queries_test.go @@ -89,7 +89,7 @@ func TestScheduledQueriesAuth(t *testing.T) { _, err := svc.GetScheduledQueriesInPack(ctx, 1, fleet.ListOptions{}) checkAuthErr(t, tt.shouldFailRead, err) - _, err = svc.ScheduleQuery(ctx, &fleet.ScheduledQuery{}) + _, err = svc.ScheduleQuery(ctx, &fleet.ScheduledQuery{Interval: 10}) checkAuthErr(t, tt.shouldFailWrite, err) _, err = svc.GetScheduledQuery(ctx, 1) @@ -112,6 +112,7 @@ func TestScheduleQuery(t *testing.T) { Name: "foobar", QueryName: "foobar", QueryID: 3, + Interval: 10, } ds.NewScheduledQueryFunc = func(ctx context.Context, q *fleet.ScheduledQuery, opts ...fleet.OptionalArg) (*fleet.ScheduledQuery, error) { @@ -132,6 +133,7 @@ func TestScheduleQueryNoName(t *testing.T) { Name: "foobar", QueryName: "foobar", QueryID: 3, + Interval: 10, } ds.QueryFunc = func(ctx context.Context, qid uint) (*fleet.Query, error) { @@ -153,7 +155,7 @@ func TestScheduleQueryNoName(t *testing.T) { _, err := svc.ScheduleQuery( test.UserContext(test.UserAdmin), - &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID}, + &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID, Interval: 10}, ) assert.NoError(t, err) assert.True(t, ds.NewScheduledQueryFuncInvoked) @@ -167,6 +169,7 @@ func TestScheduleQueryNoNameMultiple(t *testing.T) { Name: "foobar-1", QueryName: "foobar", QueryID: 3, + Interval: 10, } ds.QueryFunc = func(ctx context.Context, qid uint) (*fleet.Query, error) { @@ -177,7 +180,8 @@ func TestScheduleQueryNoNameMultiple(t *testing.T) { // No matching query return []*fleet.ScheduledQuery{ { - Name: "foobar", + Name: "foobar", + Interval: 10, }, }, nil } @@ -188,7 +192,7 @@ func TestScheduleQueryNoNameMultiple(t *testing.T) { _, err := svc.ScheduleQuery( test.UserContext(test.UserAdmin), - &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID}, + &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID, Interval: 10}, ) assert.NoError(t, err) assert.True(t, ds.NewScheduledQueryFuncInvoked) @@ -234,3 +238,119 @@ func TestFindNextNameForQuery(t *testing.T) { }) } } + +func TestScheduleQueryInterval(t *testing.T) { + ds := new(mock.Store) + svc := newTestService(t, ds, nil, nil) + + expectedQuery := &fleet.ScheduledQuery{ + Name: "foobar", + QueryName: "foobar", + QueryID: 3, + Interval: 10, + } + + ds.QueryFunc = func(ctx context.Context, qid uint) (*fleet.Query, error) { + require.Equal(t, expectedQuery.QueryID, qid) + return &fleet.Query{Name: expectedQuery.QueryName}, nil + } + ds.ListScheduledQueriesInPackWithStatsFunc = func(ctx context.Context, id uint, opts fleet.ListOptions) ([]*fleet.ScheduledQuery, error) { + // No matching query + return []*fleet.ScheduledQuery{ + { + Name: "froobling", + }, + }, nil + } + ds.NewScheduledQueryFunc = func(ctx context.Context, q *fleet.ScheduledQuery, opts ...fleet.OptionalArg) (*fleet.ScheduledQuery, error) { + assert.Equal(t, expectedQuery, q) + return expectedQuery, nil + } + + _, err := svc.ScheduleQuery( + test.UserContext(test.UserAdmin), + &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID, Interval: 10}, + ) + assert.NoError(t, err) + assert.True(t, ds.NewScheduledQueryFuncInvoked) + + // no interval + _, err = svc.ScheduleQuery( + test.UserContext(test.UserAdmin), + &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID}, + ) + assert.Error(t, err) + + // interval zero + _, err = svc.ScheduleQuery( + test.UserContext(test.UserAdmin), + &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID, Interval: 0}, + ) + assert.Error(t, err) + + // interval exceeds max + _, err = svc.ScheduleQuery( + test.UserContext(test.UserAdmin), + &fleet.ScheduledQuery{QueryID: expectedQuery.QueryID, Interval: 604801}, + ) + assert.Error(t, err) +} + +func TestModifyScheduledQueryInterval(t *testing.T) { + ds := new(mock.Store) + svc := newTestService(t, ds, nil, nil) + + ds.ScheduledQueryFunc = func(ctx context.Context, id uint) (*fleet.ScheduledQuery, error) { + assert.Equal(t, id, uint(1)) + return &fleet.ScheduledQuery{ID: id, Interval: 10}, nil + } + + testCases := []struct { + payload fleet.ScheduledQueryPayload + shouldFail bool + }{ + { + payload: fleet.ScheduledQueryPayload{ + QueryID: ptr.Uint(1), + Interval: ptr.Uint(0), + }, + shouldFail: true, + }, + { + payload: fleet.ScheduledQueryPayload{ + QueryID: ptr.Uint(1), + Interval: ptr.Uint(604801), + }, + shouldFail: true, + }, + { + payload: fleet.ScheduledQueryPayload{ + QueryID: ptr.Uint(1), + }, + shouldFail: false, + }, + { + payload: fleet.ScheduledQueryPayload{ + QueryID: ptr.Uint(1), + Interval: ptr.Uint(604800), + }, + shouldFail: false, + }, + } + + for _, tt := range testCases { + t.Run("", func(t *testing.T) { + ds.SaveScheduledQueryFunc = func(ctx context.Context, sq *fleet.ScheduledQuery) (*fleet.ScheduledQuery, error) { + assert.Equal(t, sq.ID, uint(1)) + return &fleet.ScheduledQuery{ID: sq.ID, Interval: sq.Interval}, nil + } + _, err := svc.ModifyScheduledQuery(test.UserContext(test.UserAdmin), *tt.payload.QueryID, tt.payload) + if tt.shouldFail { + assert.Error(t, err) + assert.False(t, ds.SaveScheduledQueryFuncInvoked) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/server/service/team_schedule_test.go b/server/service/team_schedule_test.go index 9535a333ff..a0828424a1 100644 --- a/server/service/team_schedule_test.go +++ b/server/service/team_schedule_test.go @@ -108,7 +108,7 @@ func TestTeamScheduleAuth(t *testing.T) { _, err := svc.GetTeamScheduledQueries(ctx, 1, fleet.ListOptions{}) checkAuthErr(t, tt.shouldFailRead, err) - _, err = svc.TeamScheduleQuery(ctx, 1, &fleet.ScheduledQuery{}) + _, err = svc.TeamScheduleQuery(ctx, 1, &fleet.ScheduledQuery{Interval: 10}) checkAuthErr(t, tt.shouldFailWrite, err) _, err = svc.ModifyTeamScheduledQueries(ctx, 1, 99, fleet.ScheduledQueryPayload{})