Add latest changes from gitlab-org/security/gitlab@16-6-stable-eev16.6.2

11 files changed, 437 insertions, 24 deletions

diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js
index 80471d8734b..9bff801dbf0 100644
--- a/spec/frontend/repository/components/table/row_spec.js
+++ b/spec/frontend/repository/components/table/row_spec.js
@@ -146,10 +146,11 @@ describe('Repository table row component', () => {
   });
 
   it.each`
-    path
-    ${'test#'}
-    ${'Änderungen'}
-  `('renders link for $path', ({ path }) => {
+    path                   | encodedPath
+    ${'test#'}             | ${'test%23'}
+    ${'Änderungen'}        | ${'%C3%84nderungen'}
+    ${'dir%2f_hello__.sh'} | ${'dir%252f_hello__.sh'}
+  `('renders link for $path', ({ path, encodedPath }) => {
     factory({
       propsData: {
         id: '1',
@@ -161,7 +162,7 @@ describe('Repository table row component', () => {
     });
 
     expect(wrapper.findComponent({ ref: 'link' }).props('to')).toBe(
-      `/-/tree/main/${encodeURIComponent(path)}?ref_type=heads`,
+      `/-/tree/main/${encodedPath}?ref_type=heads`,
     );
   });
 
diff --git a/spec/lib/gitlab/checks/tag_check_spec.rb b/spec/lib/gitlab/checks/tag_check_spec.rb
index b5aafde006f..2b1fbc7e797 100644
--- a/spec/lib/gitlab/checks/tag_check_spec.rb
+++ b/spec/lib/gitlab/checks/tag_check_spec.rb
@@ -57,6 +57,7 @@ RSpec.describe Gitlab::Checks::TagCheck, feature_category: :source_code_manageme
       context "when prohibited_tag_name_encoding_check feature flag is disabled" do
         before do
           stub_feature_flags(prohibited_tag_name_encoding_check: false)
+          allow(subject).to receive(:validate_tag_name_not_sha_like!)
         end
 
         it "doesn't prohibit tag names that include characters incompatible with UTF-8" do
@@ -71,6 +72,66 @@ RSpec.describe Gitlab::Checks::TagCheck, feature_category: :source_code_manageme
           expect { subject.validate! }.not_to raise_error
         end
       end
+
+      it "forbids SHA-1 values" do
+        allow(subject)
+          .to receive(:tag_name)
+          .and_return("267208abfe40e546f5e847444276f7d43a39503e")
+
+        expect { subject.validate! }.to raise_error(
+          Gitlab::GitAccess::ForbiddenError,
+          "You cannot create a tag with a SHA-1 or SHA-256 tag name."
+        )
+      end
+
+      it "forbids SHA-256 values" do
+        allow(subject)
+          .to receive(:tag_name)
+          .and_return("09b9fd3ea68e9b95a51b693a29568c898e27d1476bbd83c825664f18467fc175")
+
+        expect { subject.validate! }.to raise_error(
+          Gitlab::GitAccess::ForbiddenError,
+          "You cannot create a tag with a SHA-1 or SHA-256 tag name."
+        )
+      end
+
+      it "forbids '{SHA-1}{+anything}' values" do
+        allow(subject)
+          .to receive(:tag_name)
+          .and_return("267208abfe40e546f5e847444276f7d43a39503e-")
+
+        expect { subject.validate! }.to raise_error(
+          Gitlab::GitAccess::ForbiddenError,
+          "You cannot create a tag with a SHA-1 or SHA-256 tag name."
+        )
+      end
+
+      it "forbids '{SHA-256}{+anything} values" do
+        allow(subject)
+          .to receive(:tag_name)
+          .and_return("09b9fd3ea68e9b95a51b693a29568c898e27d1476bbd83c825664f18467fc175-")
+
+        expect { subject.validate! }.to raise_error(
+          Gitlab::GitAccess::ForbiddenError,
+          "You cannot create a tag with a SHA-1 or SHA-256 tag name."
+        )
+      end
+
+      it "allows SHA-1 values to be appended to the tag name" do
+        allow(subject)
+          .to receive(:tag_name)
+          .and_return("fix-267208abfe40e546f5e847444276f7d43a39503e")
+
+        expect { subject.validate! }.not_to raise_error
+      end
+
+      it "allows SHA-256 values to be appended to the tag name" do
+        allow(subject)
+          .to receive(:tag_name)
+          .and_return("fix-09b9fd3ea68e9b95a51b693a29568c898e27d1476bbd83c825664f18467fc175")
+
+        expect { subject.validate! }.not_to raise_error
+      end
     end
 
     context 'with protected tag' do
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 6c8603d7b4c..594492a160d 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -1036,6 +1036,11 @@ RSpec.describe Issue, feature_category: :team_planning do
     end
   end
 
+  it_behaves_like 'a time trackable' do
+    let(:trackable) { create(:issue) }
+    let(:timelog) { create(:issue_timelog, issue: trackable) }
+  end
+
   it_behaves_like 'an editable mentionable' do
     subject { create(:issue, project: create(:project, :repository)) }
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index d3c32da2842..1c6a29f065f 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -2058,6 +2058,11 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev
     end
   end
 
+  it_behaves_like 'a time trackable' do
+    let(:trackable) { create(:merge_request, :simple, source_project: create(:project, :repository)) }
+    let(:timelog) { create(:merge_request_timelog, merge_request: trackable) }
+  end
+
   it_behaves_like 'an editable mentionable' do
     subject { create(:merge_request, :simple, source_project: create(:project, :repository)) }
 
diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb
index 4f2f16875b8..aee2c4ded19 100644
--- a/spec/models/timelog_spec.rb
+++ b/spec/models/timelog_spec.rb
@@ -46,6 +46,53 @@ RSpec.describe Timelog, feature_category: :team_planning do
       expect(subject).to be_valid
     end
 
+    describe 'check if total time spent would be within the set range' do
+      let_it_be(:time_already_spent) { 1.minute.to_i }
+
+      before_all do
+        create(:issue_timelog, issue: issue, time_spent: time_already_spent)
+      end
+
+      it 'is valid when a negative time spent offsets the time already spent' do
+        timelog = build(:issue_timelog, issue: issue, time_spent: -time_already_spent)
+
+        expect(timelog).to be_valid
+      end
+
+      context 'when total time spent is within the allowed range' do
+        before_all do
+          # Offset the time already spent
+          create(:issue_timelog, issue: issue, time_spent: -time_already_spent)
+        end
+
+        it 'is valid' do
+          timelog = build(:issue_timelog, issue: issue, time_spent: 1.minute.to_i)
+
+          expect(timelog).to be_valid
+        end
+      end
+
+      context 'when total time spent is outside the allowed range' do
+        it 'adds an error if total time spent would exceed a year' do
+          time_to_spend = described_class::MAX_TOTAL_TIME_SPENT - time_already_spent + 1.second.to_i
+          timelog = build(:issue_timelog, issue: issue, time_spent: time_to_spend)
+
+          expect { timelog.save! }
+            .to raise_error(ActiveRecord::RecordInvalid,
+              _('Validation failed: Total time spent cannot exceed a year.'))
+        end
+
+        it 'adds an error if total time spent would be negative' do
+          time_to_spend = -time_already_spent - 1.second.to_i
+          timelog = build(:issue_timelog, issue: issue, time_spent: time_to_spend)
+
+          expect { timelog.save! }
+            .to raise_error(ActiveRecord::RecordInvalid,
+              _('Validation failed: Total time spent cannot be negative.'))
+        end
+      end
+    end
+
     describe 'when importing' do
       it 'is valid if issue_id and merge_request_id are missing' do
         subject.attributes = { issue: nil, merge_request: nil, importing: true }
diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb
index 01e02651a64..f0282b3a675 100644
--- a/spec/requests/api/resource_access_tokens_spec.rb
+++ b/spec/requests/api/resource_access_tokens_spec.rb
@@ -481,25 +481,75 @@ RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do
 
       let(:path) { "/#{source_type}s/#{resource_id}/access_tokens/#{token_id}/rotate" }
 
-      before do
-        resource.add_maintainer(project_bot)
-        resource.add_owner(user)
+      subject(:rotate_token) { post(api(path, user), params: params) }
+
+      context 'when user is owner' do
+        before do
+          resource.add_maintainer(project_bot)
+          resource.add_owner(user)
+        end
+
+        it "allows owner to rotate token", :freeze_time do
+          rotate_token
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['token']).not_to eq(token.token)
+          expect(json_response['expires_at']).to eq((Date.today + 1.week).to_s)
+        end
       end
 
-      subject(:rotate_token) { post(api(path, user), params: params) }
+      context 'when user is maintainer' do
+        before do
+          resource.add_maintainer(user)
+        end
+
+        context "when token has owner access level" do
+          let(:error_message) { 'Not eligible to rotate token with access level higher than the user' }
 
-      it "allows owner to rotate token", :freeze_time do
-        rotate_token
+          before do
+            resource.add_owner(project_bot)
+          end
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response['token']).not_to eq(token.token)
-        expect(json_response['expires_at']).to eq((Date.today + 1.week).to_s)
+          it "raises error" do
+            rotate_token
+
+            if source_type == 'project'
+              expect(response).to have_gitlab_http_status(:bad_request)
+              expect(json_response['message']).to eq("400 Bad request - #{error_message}")
+            else
+              expect(response).to have_gitlab_http_status(:unauthorized)
+            end
+          end
+        end
+
+        context 'when token has maintainer access level' do
+          before do
+            resource.add_maintainer(project_bot)
+          end
+
+          it "rotates token", :freeze_time do
+            rotate_token
+
+            if source_type == 'project'
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(json_response['token']).not_to eq(token.token)
+              expect(json_response['expires_at']).to eq((Date.today + 1.week).to_s)
+            else
+              expect(response).to have_gitlab_http_status(:unauthorized)
+            end
+          end
+        end
       end
 
       context 'when expiry is defined' do
         let(:expiry_date) { Date.today + 1.month }
         let(:params) { { expires_at: expiry_date } }
 
+        before do
+          resource.add_maintainer(project_bot)
+          resource.add_owner(user)
+        end
+
         it "allows owner to rotate token", :freeze_time do
           rotate_token
 
@@ -510,6 +560,11 @@ RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do
       end
 
       context 'without permission' do
+        before do
+          resource.add_maintainer(project_bot)
+          resource.add_owner(user)
+        end
+
         it 'returns an error message' do
           another_user = create(:user)
           resource.add_developer(another_user)
@@ -522,10 +577,21 @@ RSpec.describe API::ResourceAccessTokens, feature_category: :system_access do
 
       context 'when service raises an error' do
         let(:error_message) { 'boom!' }
+        let(:personal_token_service) { PersonalAccessTokens::RotateService }
+        let(:project_token_service) { ProjectAccessTokens::RotateService }
 
         before do
-          allow_next_instance_of(PersonalAccessTokens::RotateService) do |service|
-            allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message))
+          resource.add_maintainer(project_bot)
+          resource.add_owner(user)
+
+          if source_type == 'project'
+            allow_next_instance_of(project_token_service) do |service|
+              allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message))
+            end
+          else
+            allow_next_instance_of(personal_token_service) do |service|
+              allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message))
+            end
           end
         end
 
diff --git a/spec/services/personal_access_tokens/rotate_service_spec.rb b/spec/services/personal_access_tokens/rotate_service_spec.rb
index 522506870f6..a114f2cd909 100644
--- a/spec/services/personal_access_tokens/rotate_service_spec.rb
+++ b/spec/services/personal_access_tokens/rotate_service_spec.rb
@@ -8,16 +8,20 @@ RSpec.describe PersonalAccessTokens::RotateService, feature_category: :system_ac
 
     subject(:response) { described_class.new(token.user, token).execute }
 
-    it "rotates user's own token", :freeze_time do
-      expect(response).to be_success
+    shared_examples_for 'rotates token succesfully' do
+      it "rotates user's own token", :freeze_time do
+        expect(response).to be_success
 
-      new_token = response.payload[:personal_access_token]
+        new_token = response.payload[:personal_access_token]
 
-      expect(new_token.token).not_to eq(token.token)
-      expect(new_token.expires_at).to eq(Date.today + 1.week)
-      expect(new_token.user).to eq(token.user)
+        expect(new_token.token).not_to eq(token.token)
+        expect(new_token.expires_at).to eq(Date.today + 1.week)
+        expect(new_token.user).to eq(token.user)
+      end
     end
 
+    it_behaves_like "rotates token succesfully"
+
     it 'revokes the previous token' do
       expect { response }.to change { token.reload.revoked? }.from(false).to(true)
 
diff --git a/spec/services/project_access_tokens/rotate_service_spec.rb b/spec/services/project_access_tokens/rotate_service_spec.rb
new file mode 100644
index 00000000000..10e29be4979
--- /dev/null
+++ b/spec/services/project_access_tokens/rotate_service_spec.rb
@@ -0,0 +1,189 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+RSpec.describe ProjectAccessTokens::RotateService, feature_category: :system_access do
+  describe '#execute' do
+    let_it_be(:token, reload: true) { create(:personal_access_token) }
+    let(:current_user) { create(:user) }
+    let(:project) { create(:project, group: create(:group)) }
+    let(:error_message) { 'Not eligible to rotate token with access level higher than the user' }
+
+    subject(:response) { described_class.new(current_user, token, project).execute }
+
+    shared_examples_for 'rotates token succesfully' do
+      it "rotates user's own token", :freeze_time do
+        expect(response).to be_success
+
+        new_token = response.payload[:personal_access_token]
+
+        expect(new_token.token).not_to eq(token.token)
+        expect(new_token.expires_at).to eq(Date.today + 1.week)
+        expect(new_token.user).to eq(token.user)
+      end
+    end
+
+    context 'when user tries to rotate token with different access level' do
+      before do
+        project.add_guest(token.user)
+      end
+
+      context 'when current user is an owner' do
+        before do
+          project.add_owner(current_user)
+        end
+
+        it_behaves_like "rotates token succesfully"
+
+        context 'when creating the new token fails' do
+          let(:error_message) { 'boom!' }
+
+          before do
+            allow_next_instance_of(PersonalAccessToken) do |token|
+              allow(token).to receive_message_chain(:errors, :full_messages, :to_sentence).and_return(error_message)
+              allow(token).to receive_message_chain(:errors, :clear)
+              allow(token).to receive_message_chain(:errors, :empty?).and_return(false)
+            end
+          end
+
+          it 'returns an error' do
+            expect(response).to be_error
+            expect(response.message).to eq(error_message)
+          end
+
+          it 'reverts the changes' do
+            expect { response }.not_to change { token.reload.revoked? }.from(false)
+          end
+        end
+      end
+
+      context 'when current user is not an owner' do
+        context 'when current user is maintainer' do
+          before do
+            project.add_maintainer(current_user)
+          end
+
+          context 'when access level is not owner' do
+            it_behaves_like "rotates token succesfully"
+          end
+
+          context 'when access level is owner' do
+            before do
+              project.add_owner(token.user)
+            end
+
+            it "does not rotate token with higher priviledge" do
+              response
+
+              expect(response).to be_error
+              expect(response.message).to eq(error_message)
+            end
+          end
+        end
+
+        context 'when current user is not maintainer' do
+          before do
+            project.add_developer(current_user)
+          end
+
+          it 'does not rotate the token' do
+            response
+
+            expect(response).to be_error
+            expect(response.message).to eq(error_message)
+          end
+        end
+      end
+
+      context 'when current user is admin' do
+        let(:current_user) { create(:admin) }
+
+        context 'when admin mode enabled', :enable_admin_mode do
+          it_behaves_like "rotates token succesfully"
+        end
+
+        context 'when admin mode not enabled' do
+          it 'does not rotate the token' do
+            response
+
+            expect(response).to be_error
+            expect(response.message).to eq(error_message)
+          end
+        end
+      end
+
+      context 'when nested membership' do
+        let_it_be(:project_bot) { create(:user, :project_bot) }
+        let(:token) { create(:personal_access_token, user: project_bot) }
+        let(:top_level_group) { create(:group) }
+        let(:sub_group) { create(:group, parent: top_level_group) }
+        let(:project) { create(:project, group: sub_group) }
+
+        before do
+          project.add_maintainer(project_bot)
+        end
+
+        context 'when current user is an owner' do
+          before do
+            project.add_owner(current_user)
+          end
+
+          it_behaves_like "rotates token succesfully"
+
+          context 'when its a bot user' do
+            let_it_be(:bot_user) { create(:user, :project_bot) }
+            let_it_be(:bot_user_membership) do
+              create(:project_member, :developer, user: bot_user, project: create(:project))
+            end
+
+            let_it_be(:token, reload: true) { create(:personal_access_token, user: bot_user) }
+
+            it 'updates membership expires at' do
+              response
+
+              new_token = response.payload[:personal_access_token]
+              expect(bot_user_membership.reload.expires_at).to eq(new_token.expires_at)
+            end
+          end
+        end
+
+        context 'when current user is not an owner' do
+          context 'when current user is maintainer' do
+            before do
+              project.add_maintainer(current_user)
+            end
+
+            context 'when access level is not owner' do
+              it_behaves_like "rotates token succesfully"
+            end
+
+            context 'when access level is owner' do
+              before do
+                project.add_owner(token.user)
+              end
+
+              it "does not rotate token with higher priviledge" do
+                response
+
+                expect(response).to be_error
+                expect(response.message).to eq(error_message)
+              end
+            end
+          end
+
+          context 'when current user is not maintainer' do
+            before do
+              project.add_developer(current_user)
+            end
+
+            it 'does not rotate the token' do
+              response
+
+              expect(response).to be_error
+              expect(response.message).to eq(error_message)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/system_notes/time_tracking_service_spec.rb b/spec/services/system_notes/time_tracking_service_spec.rb
index 3242ae9e533..cf994220946 100644
--- a/spec/services/system_notes/time_tracking_service_spec.rb
+++ b/spec/services/system_notes/time_tracking_service_spec.rb
@@ -272,7 +272,11 @@ RSpec.describe ::SystemNotes::TimeTrackingService, feature_category: :team_plann
     context 'when the timelog has a negative time spent value' do
       let_it_be(:noteable, reload: true) { create(:issue, project: project) }
 
-      let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -1800, spent_at: '2022-03-30T00:00:00.000Z') }
+      let!(:existing_timelog) { create(:timelog, user: author, issue: noteable, time_spent: time_spent.to_i) }
+
+      let(:time_spent) { 1800.seconds }
+      let(:spent_at) { '2022-03-30T00:00:00.000Z' }
+      let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -time_spent.to_i, spent_at: spent_at) }
 
       it 'sets the note text' do
         expect(subject.note).to eq "subtracted 30m of time spent at 2022-03-30"
@@ -296,7 +300,11 @@ RSpec.describe ::SystemNotes::TimeTrackingService, feature_category: :team_plann
     context 'when the timelog has a negative time spent value' do
       let_it_be(:noteable, reload: true) { create(:issue, project: project) }
 
-      let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -1800, spent_at: '2022-03-30T00:00:00.000Z') }
+      let!(:existing_timelog) { create(:timelog, user: author, issue: noteable, time_spent: time_spent.to_i) }
+
+      let(:time_spent) { 1800.seconds }
+      let(:spent_at) { '2022-03-30T00:00:00.000Z' }
+      let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -time_spent.to_i, spent_at: spent_at) }
 
       it 'sets the note text' do
         expect(subject.note).to eq "deleted -30m of spent time from 2022-03-30"
diff --git a/spec/support/shared_examples/models/trackable_shared_examples.rb b/spec/support/shared_examples/models/trackable_shared_examples.rb
new file mode 100644
index 00000000000..649a8eb2d6c
--- /dev/null
+++ b/spec/support/shared_examples/models/trackable_shared_examples.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'a time trackable' do
+  describe '#total_time_spent' do
+    context 'when total time spent exceeds the allowed limit' do
+      let(:time_spent) { Timelog::MAX_TOTAL_TIME_SPENT + 1.second }
+
+      it 'returns the maximum allowed total time spent' do
+        timelog.update_column(:time_spent, time_spent.to_i)
+
+        expect(trackable.total_time_spent).to eq(Timelog::MAX_TOTAL_TIME_SPENT)
+      end
+
+      context 'when total time spent is below 0' do
+        let(:time_spent) { -Timelog::MAX_TOTAL_TIME_SPENT - 1.second }
+
+        it 'returns the minimum allowed total time spent' do
+          timelog.update_column(:time_spent, time_spent.to_i)
+
+          expect(trackable.total_time_spent).to eq(-Timelog::MAX_TOTAL_TIME_SPENT)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb
index dec15cb68b3..eddda969ba2 100644
--- a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb
+++ b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb
@@ -169,6 +169,8 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
     end
 
     it "resets spent time for #{issuable_name}" do
+      issuable.update!(spend_time: { duration: 60, user_id: user.id })
+
       travel_to(2.minutes.from_now) do
         expect do
           post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_spent_time", user)