fix: enforce lower and upper limits on template max_ttl_ms

johnstcn · johnstcn · commit e488e13082d7 · 2022-08-24T09:50:28.000Z
diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -49,6 +49,7 @@ func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objec
 // This function will log appropriately, but the caller must return an
 // error to the api client.
 // Eg:
+//
 //	if !h.Authorize(...) {
 //		httpapi.Forbidden(rw)
 //		return
diff --git a/coderd/templates.go b/coderd/templates.go
@@ -176,6 +176,25 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
 	if !ptr.NilOrZero(createTemplate.MaxTTLMillis) {
 		maxTTL = time.Duration(*createTemplate.MaxTTLMillis) * time.Millisecond
 	}
+	if maxTTL < 0 {
+		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+			Message: "Invalid create template request.",
+			Validations: []codersdk.ValidationError{
+				{Field: "max_ttl_ms", Detail: "Must be a positive integer."},
+			},
+		})
+		return
+	}
+
+	if maxTTL > maxTTLDefault {
+		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+			Message: "Invalid create template request.",
+			Validations: []codersdk.ValidationError{
+				{Field: "max_ttl_ms", Detail: "Cannot be greater than " + maxTTLDefault.String()},
+			},
+		})
+		return
+	}
 
 	minAutostartInterval := minAutostartIntervalDefault
 	if !ptr.NilOrZero(createTemplate.MinAutostartIntervalMillis) {
@@ -384,6 +403,15 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
 	if req.MinAutostartIntervalMillis < 0 {
 		validErrs = append(validErrs, codersdk.ValidationError{Field: "min_autostart_interval_ms", Detail: "Must be a positive integer."})
 	}
+	if req.MaxTTLMillis > maxTTLDefault.Milliseconds() {
+		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
+			Message: "Invalid create template request.",
+			Validations: []codersdk.ValidationError{
+				{Field: "max_ttl_ms", Detail: "Cannot be greater than " + maxTTLDefault.String()},
+			},
+		})
+		return
+	}
 
 	if len(validErrs) > 0 {
 		httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
diff --git a/coderd/templates_test.go b/coderd/templates_test.go
@@ -108,6 +108,46 @@ func TestPostTemplateByOrganization(t *testing.T) {
 		require.Equal(t, http.StatusConflict, apiErr.StatusCode())
 	})
 
+	t.Run("MaxTTLTooLow", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
+			Name:         "testing",
+			VersionID:    version.ID,
+			MaxTTLMillis: ptr.Ref(int64(-1)),
+		})
+		var apiErr *codersdk.Error
+		require.ErrorAs(t, err, &apiErr)
+		require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
+		require.Contains(t, err.Error(), "max_ttl_ms: Must be a positive integer")
+	})
+
+	t.Run("MaxTTLTooHigh", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.CreateTemplate(ctx, user.OrganizationID, codersdk.CreateTemplateRequest{
+			Name:         "testing",
+			VersionID:    version.ID,
+			MaxTTLMillis: ptr.Ref(365 * 24 * time.Hour.Milliseconds()),
+		})
+		var apiErr *codersdk.Error
+		require.ErrorAs(t, err, &apiErr)
+		require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
+		require.Contains(t, err.Error(), "max_ttl_ms: Cannot be greater than")
+	})
+
 	t.Run("Unauthorized", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
@@ -271,6 +311,58 @@ func TestPatchTemplateMeta(t *testing.T) {
 		assert.Equal(t, req.MinAutostartIntervalMillis, updated.MinAutostartIntervalMillis)
 	})
 
+	t.Run("MaxTTLTooLow", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.MaxTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
+		})
+		req := codersdk.UpdateTemplateMeta{
+			MaxTTLMillis: -1,
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
+		require.ErrorContains(t, err, "max_ttl_ms: Must be a positive integer")
+
+		// Ensure no update occurred
+		updated, err := client.Template(ctx, template.ID)
+		require.NoError(t, err)
+		assert.Equal(t, updated.UpdatedAt, template.UpdatedAt)
+		assert.Equal(t, updated.MaxTTLMillis, template.MaxTTLMillis)
+	})
+
+	t.Run("MaxTTLTooHigh", func(t *testing.T) {
+		t.Parallel()
+
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.MaxTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
+		})
+		req := codersdk.UpdateTemplateMeta{
+			MaxTTLMillis: 365 * 24 * time.Hour.Milliseconds(),
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		_, err := client.UpdateTemplateMeta(ctx, template.ID, req)
+		require.ErrorContains(t, err, "max_ttl_ms: Cannot be greater than")
+
+		// Ensure no update occurred
+		updated, err := client.Template(ctx, template.ID)
+		require.NoError(t, err)
+		assert.Equal(t, updated.UpdatedAt, template.UpdatedAt)
+		assert.Equal(t, updated.MaxTTLMillis, template.MaxTTLMillis)
+	})
+
 	t.Run("NotModified", func(t *testing.T) {
 		t.Parallel()
 
