Add validation for pack scheduled query interval (#5918)

changes/issue-5624-packs-interval

@@ -0,0 +1 @@
+* Added validation for pack scheduled query interval

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) {

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<string>(
     editQuery?.interval.toString() || ""
   );
+  const [errorFrequency, setErrorFrequency] = useState<string>("");
   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 = ({
         )}
         <InputField
           onChange={onChangeFrequency}
+          error={errorFrequency}
           inputWrapperClass={`${baseClass}__form-field ${baseClass}__form-field--frequency`}
           value={selectedFrequency}
           placeholder="- - -"

frontend/utilities/constants.ts

@@ -163,6 +163,8 @@ export const LOGGING_TYPE_OPTIONS = [
   },
 ];
 
+export const MAX_OSQUERY_SCHEDULED_QUERY_INTERVAL = 604800;
+
 export const MIN_OSQUERY_VERSION_OPTIONS = [
   { label: "All", value: "" },
   { label: "4.7.0 +", value: "4.7.0" },

server/fleet/packs.go

@@ -8,6 +8,9 @@ import (
 	"github.com/fleetdm/fleet/v4/server/ptr"
 )
 
+// MaxScheduledQueryInterval is the maximum interval value (in seconds) allowed by osquery
+const MaxScheduledQueryInterval = 604800
+
 type PackListOptions struct {
 	ListOptions
 
@@ -110,7 +113,10 @@ type PackPayload struct {
 	TeamIDs     *[]uint `json:"team_ids"`
 }
 
-var errPackEmptyName = errors.New("pack name cannot be empty")
+var (
+	errPackEmptyName       = errors.New("pack name cannot be empty")
+	errPackInvalidInterval = errors.New("pack scheduled query interval must be an integer greater than 1 and less than 604800")
+)
 
 // Verify verifies the pack's payload fields are valid.
 func (p *PackPayload) Verify() error {
@@ -137,6 +143,11 @@ func (p *PackSpec) Verify() error {
 	if emptyString(p.Name) {
 		return errPackEmptyName
 	}
+	for _, sq := range p.Queries {
+		if sq.Interval < 1 || sq.Interval > MaxScheduledQueryInterval {
+			return errPackInvalidInterval
+		}
+	}
 	return nil
 }
 

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{})

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")

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)
+			}
+		})
+	}
+}

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{})