Merge branch 'fix/gb/fix-project-settings-build-time-validation' into 'master'

Improve project build time setting validation

Closes #44443

See merge request gitlab-org/gitlab-ce!20591

3 files changed, 17 insertions, 8 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 1894de6ceed..e29bca365a4 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -368,8 +368,10 @@ class Project < ActiveRecord::Base
   chronic_duration_attr :build_timeout_human_readable, :build_timeout, default: 3600
 
   validates :build_timeout, allow_nil: true,
-                            numericality: { greater_than_or_equal_to: 600,
-                                            message: 'needs to be at least 10 minutes' }
+                            numericality: { greater_than_or_equal_to: 10.minutes,
+                                            less_than: 1.month,
+                                            only_integer: true,
+                                            message: 'needs to be beetween 10 minutes and 1 month' }
 
   # Returns a collection of projects that is either public or visible to the
   # logged in user.
diff --git a/changelogs/unreleased/fix-gb-fix-project-settings-build-time-validation.yml b/changelogs/unreleased/fix-gb-fix-project-settings-build-time-validation.yml
new file mode 100644
index 00000000000..adf582e34a2
--- /dev/null
+++ b/changelogs/unreleased/fix-gb-fix-project-settings-build-time-validation.yml
@@ -0,0 +1,5 @@
+---
+title: Limit maximum project build timeout setting to 1 month
+merge_request: 20591
+author:
+type: fixed
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index c01c7bc47b5..d200e5f2e42 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -149,23 +149,25 @@ describe Project do
     it { is_expected.to validate_presence_of(:name) }
     it { is_expected.to validate_uniqueness_of(:name).scoped_to(:namespace_id) }
     it { is_expected.to validate_length_of(:name).is_at_most(255) }
-
     it { is_expected.to validate_presence_of(:path) }
     it { is_expected.to validate_length_of(:path).is_at_most(255) }
-
     it { is_expected.to validate_length_of(:description).is_at_most(2000) }
-
     it { is_expected.to validate_length_of(:ci_config_path).is_at_most(255) }
     it { is_expected.to allow_value('').for(:ci_config_path) }
     it { is_expected.not_to allow_value('test/../foo').for(:ci_config_path) }
     it { is_expected.not_to allow_value('/test/foo').for(:ci_config_path) }
-
     it { is_expected.to validate_presence_of(:creator) }
-
     it { is_expected.to validate_presence_of(:namespace) }
-
     it { is_expected.to validate_presence_of(:repository_storage) }
 
+    it 'validates build timeout constraints' do
+      is_expected.to validate_numericality_of(:build_timeout)
+        .only_integer
+        .is_greater_than_or_equal_to(10.minutes)
+        .is_less_than(1.month)
+        .with_message('needs to be beetween 10 minutes and 1 month')
+    end
+
     it 'does not allow new projects beyond user limits' do
       project2 = build(:project)
       allow(project2).to receive(:creator).and_return(double(can_create_project?: false, projects_limit: 0).as_null_object)