From 297ad29837559abc899e81626ab9deeab11c1c2c Mon Sep 17 00:00:00 2001
From: GitLab Bot
Date: Wed, 29 Mar 2023 23:47:44 +0000
Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee
---
.../environments/environment_names_finder.rb | 11 +--------
app/models/project_feature.rb | 3 ++-
.../explore/projects/page_out_of_bounds.html.haml | 2 +-
.../environments/environment_names_finder_spec.rb | 26 +++++++++++++++++-----
spec/policies/project_policy_spec.rb | 4 ++--
.../projects/page_out_of_bounds.html.haml_spec.rb | 26 ++++++++++++++++++++++
6 files changed, 53 insertions(+), 19 deletions(-)
create mode 100644 spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb
diff --git a/app/finders/environments/environment_names_finder.rb b/app/finders/environments/environment_names_finder.rb
index d4928f0fc84..ffb689f45e2 100644
--- a/app/finders/environments/environment_names_finder.rb
+++ b/app/finders/environments/environment_names_finder.rb
@@ -32,18 +32,9 @@ module Environments
end
def namespace_environments
- # We assume reporter access is needed for the :read_environment permission
- # here. This expection is also present in
- # IssuableFinder::Params#min_access_level, which is used for filtering out
- # merge requests that don't have the right permissions.
- #
- # We use this approach so we don't need to load every project into memory
- # just to verify if we can see their environments. Doing so would not be
- # efficient, and possibly mess up pagination if certain projects are not
- # meant to be visible.
projects = project_or_group
.all_projects
- .public_or_visible_to_user(current_user, Gitlab::Access::REPORTER)
+ .filter_by_feature_visibility(:environments, current_user)
Environment.for_project(projects)
end
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index 11f4a3f3b6f..d7bb9fa18ae 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -36,7 +36,8 @@ class ProjectFeature < ApplicationRecord
merge_requests: Gitlab::Access::REPORTER,
metrics_dashboard: Gitlab::Access::REPORTER,
container_registry: Gitlab::Access::REPORTER,
- package_registry: Gitlab::Access::REPORTER
+ package_registry: Gitlab::Access::REPORTER,
+ environments: Gitlab::Access::REPORTER
}.freeze
PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
diff --git a/app/views/explore/projects/page_out_of_bounds.html.haml b/app/views/explore/projects/page_out_of_bounds.html.haml
index ef5ee2c679e..e13768a3ccb 100644
--- a/app/views/explore/projects/page_out_of_bounds.html.haml
+++ b/app/views/explore/projects/page_out_of_bounds.html.haml
@@ -18,5 +18,5 @@
%h5= _("Maximum page reached")
%p= _("Sorry, you have exceeded the maximum browsable page number. Please use the API to explore further.")
- = render Pajamas::ButtonComponent.new(href: request.params.merge(page: @max_page_number)) do
+ = render Pajamas::ButtonComponent.new(href: safe_params.merge(page: @max_page_number)) do
= _("Back to page %{number}") % { number: @max_page_number }
diff --git a/spec/finders/environments/environment_names_finder_spec.rb b/spec/finders/environments/environment_names_finder_spec.rb
index 438f9e9ea7c..c2336c59119 100644
--- a/spec/finders/environments/environment_names_finder_spec.rb
+++ b/spec/finders/environments/environment_names_finder_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
describe '#execute' do
let!(:group) { create(:group) }
let!(:public_project) { create(:project, :public, namespace: group) }
+ let_it_be_with_reload(:public_project_with_private_environments) { create(:project, :public) }
let!(:private_project) { create(:project, :private, namespace: group) }
let!(:user) { create(:user) }
@@ -14,6 +15,11 @@ RSpec.describe Environments::EnvironmentNamesFinder do
create(:environment, name: 'gprd', project: public_project)
create(:environment, name: 'gprd', project: private_project)
create(:environment, name: 'gcny', project: private_project)
+ create(:environment, name: 'gprivprd', project: public_project_with_private_environments)
+ create(:environment, name: 'gprivstg', project: public_project_with_private_environments)
+
+ public_project_with_private_environments.update!(namespace: group)
+ public_project_with_private_environments.project_feature.update!(environments_access_level: Featurable::PRIVATE)
end
context 'using a group' do
@@ -23,7 +29,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
names = described_class.new(group, user).execute
- expect(names).to eq(%w[gcny gprd gstg])
+ expect(names).to eq(%w[gcny gprd gprivprd gprivstg gstg])
end
end
@@ -33,7 +39,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
names = described_class.new(group, user).execute
- expect(names).to eq(%w[gcny gprd gstg])
+ expect(names).to eq(%w[gcny gprd gprivprd gprivstg gstg])
end
end
@@ -57,8 +63,18 @@ RSpec.describe Environments::EnvironmentNamesFinder do
end
end
+ context 'with a public project reporter which has private environments' do
+ it 'returns environment names for public projects' do
+ public_project_with_private_environments.add_reporter(user)
+
+ names = described_class.new(group, user).execute
+
+ expect(names).to eq(%w[gprd gprivprd gprivstg gstg])
+ end
+ end
+
context 'with a group guest' do
- it 'returns environment names for all public projects' do
+ it 'returns environment names for public projects' do
group.add_guest(user)
names = described_class.new(group, user).execute
@@ -68,7 +84,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
end
context 'with a non-member' do
- it 'returns environment names for all public projects' do
+ it 'returns environment names for only public projects with public environments' do
names = described_class.new(group, user).execute
expect(names).to eq(%w[gprd gstg])
@@ -76,7 +92,7 @@ RSpec.describe Environments::EnvironmentNamesFinder do
end
context 'without a user' do
- it 'returns environment names for all public projects' do
+ it 'returns environment names for only public projects with public environments' do
names = described_class.new(group).execute
expect(names).to eq(%w[gprd gstg])
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index a98f091b9fc..2a4a169e5c5 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -2016,7 +2016,7 @@ RSpec.describe ProjectPolicy, feature_category: :authentication_and_authorizatio
:public | ProjectFeature::ENABLED | :anonymous | true
:public | ProjectFeature::PRIVATE | :maintainer | true
:public | ProjectFeature::PRIVATE | :developer | true
- :public | ProjectFeature::PRIVATE | :guest | true
+ :public | ProjectFeature::PRIVATE | :guest | false
:public | ProjectFeature::PRIVATE | :anonymous | false
:public | ProjectFeature::DISABLED | :maintainer | false
:public | ProjectFeature::DISABLED | :developer | false
@@ -2028,7 +2028,7 @@ RSpec.describe ProjectPolicy, feature_category: :authentication_and_authorizatio
:internal | ProjectFeature::ENABLED | :anonymous | false
:internal | ProjectFeature::PRIVATE | :maintainer | true
:internal | ProjectFeature::PRIVATE | :developer | true
- :internal | ProjectFeature::PRIVATE | :guest | true
+ :internal | ProjectFeature::PRIVATE | :guest | false
:internal | ProjectFeature::PRIVATE | :anonymous | false
:internal | ProjectFeature::DISABLED | :maintainer | false
:internal | ProjectFeature::DISABLED | :developer | false
diff --git a/spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb b/spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb
new file mode 100644
index 00000000000..1ace28be5b4
--- /dev/null
+++ b/spec/views/explore/projects/page_out_of_bounds.html.haml_spec.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'explore/projects/page_out_of_bounds.html.haml', feature_category: :projects do
+ let(:page_limit) { 10 }
+ let(:unsafe_param) { 'hacked_using_unsafe_param!' }
+
+ before do
+ assign(:max_page_number, page_limit)
+
+ controller.params[:action] = 'index'
+ controller.params[:host] = unsafe_param
+ controller.params[:protocol] = unsafe_param
+ controller.params[:sort] = 'name_asc'
+ end
+
+ it 'removes unsafe params from the link' do
+ render
+
+ href = "/explore/projects?page=#{page_limit}&sort=name_asc"
+ button_text = format(_("Back to page %{number}"), number: page_limit)
+ expect(rendered).to have_link(button_text, href: href)
+ expect(rendered).not_to include(unsafe_param)
+ end
+end
--
cgit v1.2.1
From ea1912e51181abbbad5f33486ac5f455c19f1d2e Mon Sep 17 00:00:00 2001
From: GitLab Bot
Date: Wed, 29 Mar 2023 23:52:20 +0000
Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee
---
app/assets/javascripts/pages/projects/project.js | 3 +-
app/assets/javascripts/repository/index.js | 9 ++-
.../repository/utils/ref_switcher_utils.js | 30 +++++---
app/controllers/concerns/confirm_email_warning.rb | 14 +++-
app/controllers/projects/blob_controller.rb | 10 +++
app/controllers/projects/refs_controller.rb | 2 +-
app/controllers/projects/tree_controller.rb | 9 +++
app/controllers/projects_controller.rb | 10 ++-
app/finders/notes_finder.rb | 8 ++
app/models/hooks/web_hook.rb | 18 ++++-
app/policies/project_policy.rb | 1 -
.../merge_requests/push_options_handler_service.rb | 10 ++-
app/views/projects/tree/_tree_header.html.haml | 2 +-
..._nullify_last_error_from_project_mirror_data.rb | 26 +++++++
db/schema_migrations/20230208131808 | 1 +
.../output_example_snapshots/snapshot_spec.html | 16 ++--
lib/api/repositories.rb | 4 +
lib/extracts_ref.rb | 14 +++-
.../nullify_last_error_from_project_mirror_data.rb | 17 ++++
lib/gitlab/unicode.rb | 6 ++
lib/gitlab/url_sanitizer.rb | 90 ++++++++++++++--------
lib/rouge/formatters/html_gitlab.rb | 9 ++-
locale/gitlab.pot | 3 +
spec/controllers/admin/hooks_controller_spec.rb | 3 +-
.../concerns/confirm_email_warning_spec.rb | 34 +++++++-
spec/controllers/projects/blob_controller_spec.rb | 33 +++++++-
.../projects/clusters_controller_spec.rb | 23 +++++-
.../environments/prometheus_api_controller_spec.rb | 23 +++++-
spec/controllers/projects/refs_controller_spec.rb | 4 +-
spec/controllers/projects/tree_controller_spec.rb | 37 +++++++--
spec/controllers/projects_controller_spec.rb | 63 +++++++++++++++
spec/factories/project_hooks.rb | 2 +-
spec/features/admin/users/user_spec.rb | 30 ++++++++
spec/finders/notes_finder_spec.rb | 20 +++++
.../repository/utils/ref_switcher_utils_spec.js | 31 +++++++-
spec/helpers/hooks_helper_spec.rb | 2 +-
...ify_last_error_from_project_mirror_data_spec.rb | 84 ++++++++++++++++++++
spec/lib/gitlab/url_sanitizer_spec.rb | 31 +++++---
spec/lib/rouge/formatters/html_gitlab_spec.rb | 21 ++++-
...ify_last_error_from_project_mirror_data_spec.rb | 37 +++++++++
spec/models/hooks/web_hook_spec.rb | 28 +++++++
spec/policies/project_policy_spec.rb | 46 ++++++++++-
spec/requests/api/repositories_spec.rb | 16 ++++
.../push_options_handler_service_spec.rb | 15 ++++
spec/services/web_hook_service_spec.rb | 4 +-
45 files changed, 790 insertions(+), 109 deletions(-)
create mode 100644 db/post_migrate/20230208131808_nullify_last_error_from_project_mirror_data.rb
create mode 100644 db/schema_migrations/20230208131808
create mode 100644 lib/gitlab/background_migration/nullify_last_error_from_project_mirror_data.rb
create mode 100644 spec/lib/gitlab/background_migration/nullify_last_error_from_project_mirror_data_spec.rb
create mode 100644 spec/migrations/nullify_last_error_from_project_mirror_data_spec.rb
diff --git a/app/assets/javascripts/pages/projects/project.js b/app/assets/javascripts/pages/projects/project.js
index 4c9eb830ff6..27659348dcc 100644
--- a/app/assets/javascripts/pages/projects/project.js
+++ b/app/assets/javascripts/pages/projects/project.js
@@ -118,9 +118,10 @@ export default class Project {
const urlParams = { [fieldName]: ref };
if (params.group === BRANCH_GROUP_NAME) {
urlParams.ref_type = BRANCH_REF_TYPE;
- } else {
+ } else if (params.group === TAG_GROUP_NAME) {
urlParams.ref_type = TAG_REF_TYPE;
}
+
link.href = mergeUrlParams(urlParams, linkTarget);
}
diff --git a/app/assets/javascripts/repository/index.js b/app/assets/javascripts/repository/index.js
index e5d22f50d72..0f6b159ffc5 100644
--- a/app/assets/javascripts/repository/index.js
+++ b/app/assets/javascripts/repository/index.js
@@ -2,7 +2,7 @@ import { GlButton } from '@gitlab/ui';
import Vue from 'vue';
import Vuex from 'vuex';
import { parseBoolean } from '~/lib/utils/common_utils';
-import { escapeFileUrl, visitUrl } from '~/lib/utils/url_utility';
+import { joinPaths, escapeFileUrl, visitUrl } from '~/lib/utils/url_utility';
import { __ } from '~/locale';
import initWebIdeLink from '~/pages/projects/shared/web_ide_link';
import PerformancePlugin from '~/performance/vue_performance_plugin';
@@ -119,7 +119,7 @@ export default function setupVueRepositoryList() {
if (!refSwitcherEl) return false;
- const { projectId, projectRootPath } = refSwitcherEl.dataset;
+ const { projectId, projectRootPath, refType } = refSwitcherEl.dataset;
return new Vue({
el: refSwitcherEl,
@@ -127,11 +127,12 @@ export default function setupVueRepositoryList() {
return createElement(RefSelector, {
props: {
projectId,
- value: ref,
+ value: refType ? joinPaths('refs', refType, ref) : ref,
+ useSymbolicRefNames: true,
},
on: {
input(selectedRef) {
- visitUrl(generateRefDestinationPath(projectRootPath, selectedRef));
+ visitUrl(generateRefDestinationPath(projectRootPath, ref, selectedRef));
},
},
});
diff --git a/app/assets/javascripts/repository/utils/ref_switcher_utils.js b/app/assets/javascripts/repository/utils/ref_switcher_utils.js
index f296b5e9b4a..fc0d521ad54 100644
--- a/app/assets/javascripts/repository/utils/ref_switcher_utils.js
+++ b/app/assets/javascripts/repository/utils/ref_switcher_utils.js
@@ -15,22 +15,30 @@ const NAMESPACE_TARGET_REGEX = /(\/-\/(blob|tree))\/.*?\/(.*)/;
* @param {string} projectRootPath - The root path for a project.
* @param {string} selectedRef - The selected ref from the ref dropdown.
*/
-export function generateRefDestinationPath(projectRootPath, selectedRef) {
- const currentPath = window.location.pathname;
- const encodedHash = '%23';
+
+export function generateRefDestinationPath(projectRootPath, ref, selectedRef) {
+ const url = new URL(window.location.href);
+ const currentPath = url.pathname;
+ let refType = null;
let namespace = '/-/tree';
let target;
+ let actualRef = selectedRef;
+
+ const matches = selectedRef.match(/^refs\/(heads|tags)\/(.+)/);
+ if (matches) {
+ [, refType, actualRef] = matches;
+ }
+ if (refType) {
+ url.searchParams.set('ref_type', refType);
+ } else {
+ url.searchParams.delete('ref_type');
+ }
+
const match = NAMESPACE_TARGET_REGEX.exec(currentPath);
if (match) {
[, namespace, , target] = match;
}
+ url.pathname = joinPaths(projectRootPath, namespace, actualRef, target);
- const destinationPath = joinPaths(
- projectRootPath,
- namespace,
- encodeURI(selectedRef).replace(/#/g, encodedHash),
- target,
- );
-
- return `${destinationPath}${window.location.hash}`;
+ return url.toString();
}
diff --git a/app/controllers/concerns/confirm_email_warning.rb b/app/controllers/concerns/confirm_email_warning.rb
index ec5140bf223..2711c823275 100644
--- a/app/controllers/concerns/confirm_email_warning.rb
+++ b/app/controllers/concerns/confirm_email_warning.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
module ConfirmEmailWarning
+ include Gitlab::Utils::StrongMemoize
extend ActiveSupport::Concern
included do
@@ -17,11 +18,9 @@ module ConfirmEmailWarning
return unless current_user
return if current_user.confirmed?
- email = current_user.unconfirmed_email || current_user.email
-
flash.now[:warning] = format(
confirm_warning_message,
- email: email,
+ email: email_to_display,
resend_link: view_context.link_to(_('Resend it'), user_confirmation_path(user: { email: email }), method: :post),
update_link: view_context.link_to(_('Update it'), profile_path)
).html_safe
@@ -29,7 +28,16 @@ module ConfirmEmailWarning
private
+ def email
+ current_user.unconfirmed_email || current_user.email
+ end
+ strong_memoize_attr :email
+
def confirm_warning_message
_("Please check your email (%{email}) to verify that you own this address and unlock the power of CI/CD. Didn't receive it? %{resend_link}. Wrong email address? %{update_link}.")
end
+
+ def email_to_display
+ html_escape(email)
+ end
end
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 4eda76f4f21..dd2f980e880 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -31,6 +31,7 @@ class Projects::BlobController < Projects::ApplicationController
before_action :authorize_edit_tree!, only: [:new, :create, :update, :destroy]
before_action :commit, except: [:new, :create]
+ before_action :check_for_ambiguous_ref, only: [:show]
before_action :blob, except: [:new, :create]
before_action :require_branch_head, only: [:edit, :update]
before_action :editor_variables, except: [:show, :preview, :diff]
@@ -145,6 +146,15 @@ class Projects::BlobController < Projects::ApplicationController
end
end
+ def check_for_ambiguous_ref
+ @ref_type = ref_type
+
+ if @ref_type == ExtractsRef::BRANCH_REF_TYPE && ambiguous_ref?(@project, @ref)
+ branch = @project.repository.find_branch(@ref)
+ redirect_to project_blob_path(@project, File.join(branch.target, @path))
+ end
+ end
+
def commit
@commit ||= @repository.commit(@ref)
diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb
index 8ac6d872aae..0b52d03ab95 100644
--- a/app/controllers/projects/refs_controller.rb
+++ b/app/controllers/projects/refs_controller.rb
@@ -22,7 +22,7 @@ class Projects::RefsController < Projects::ApplicationController
when "tree"
project_tree_path(@project, @id)
when "blob"
- project_blob_path(@project, @id)
+ project_blob_path(@project, @id, ref_type: ref_type)
when "graph"
if Feature.enabled?(:use_ref_type_parameter, @project)
project_network_path(@project, @id, ref_type: ref_type)
diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb
index 737a6290431..1f7912e15df 100644
--- a/app/controllers/projects/tree_controller.rb
+++ b/app/controllers/projects/tree_controller.rb
@@ -28,6 +28,15 @@ class Projects::TreeController < Projects::ApplicationController
def show
return render_404 unless @commit
+ @ref_type = ref_type
+ if @ref_type == BRANCH_REF_TYPE && ambiguous_ref?(@project, @ref)
+ branch = @project.repository.find_branch(@ref)
+ if branch
+ redirect_to project_tree_path(@project, branch.target)
+ return
+ end
+ end
+
if tree.entries.empty?
if @repository.blob_at(@commit.id, @path)
redirect_to project_blob_path(@project, File.join(@ref, @path))
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index ee2c268ff33..ed7c163c108 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -172,11 +172,19 @@ class ProjectsController < Projects::ApplicationController
flash.now[:alert] = _("Project '%{project_name}' queued for deletion.") % { project_name: @project.name }
end
+ if ambiguous_ref?(@project, @ref)
+ branch = @project.repository.find_branch(@ref)
+
+ # The files view would render a ref other than the default branch
+ # This redirect can be removed once the view is fixed
+ redirect_to(project_tree_path(@project, branch.target), alert: _("The default branch of this project clashes with another ref"))
+ return
+ end
+
respond_to do |format|
format.html do
@notification_setting = current_user.notification_settings_for(@project) if current_user
@project = @project.present(current_user: current_user)
-
render_landing_page
end
diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index c542ffbce7e..ea16bc01029 100644
--- a/app/finders/notes_finder.rb
+++ b/app/finders/notes_finder.rb
@@ -30,6 +30,7 @@ class NotesFinder
notes = init_collection
notes = since_fetch_at(notes)
notes = notes.with_notes_filter(@params[:notes_filter]) if notes_filter?
+ notes = redact_internal(notes)
sort(notes)
end
@@ -181,6 +182,13 @@ class NotesFinder
notes.order_by(sort)
end
+
+ def redact_internal(notes)
+ subject = @project || target
+ return notes if Ability.allowed?(@current_user, :reporter_access, subject)
+
+ notes.not_internal
+ end
end
NotesFinder.prepend_mod_with('NotesFinder')
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 49418cda3ac..a787eb1d5f0 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -41,7 +41,7 @@ class WebHook < ApplicationRecord
after_initialize :initialize_url_variables
before_validation :reset_token
- before_validation :reset_url_variables, unless: ->(hook) { hook.is_a?(ServiceHook) }
+ before_validation :reset_url_variables, unless: ->(hook) { hook.is_a?(ServiceHook) }, on: :update
before_validation :set_branch_filter_nil, if: :branch_filter_strategy_all_branches?
validates :push_events_branch_filter, untrusted_regexp: true, if: :branch_filter_strategy_regex?
validates :push_events_branch_filter, "web_hooks/wildcard_branch_filter": true, if: :branch_filter_strategy_wildcard?
@@ -189,7 +189,7 @@ class WebHook < ApplicationRecord
# See app/validators/json_schemas/web_hooks_url_variables.json
VARIABLE_REFERENCE_RE = /\{([A-Za-z]+[0-9]*(?:[._-][A-Za-z0-9]+)*)\}/.freeze
- def interpolated_url
+ def interpolated_url(url = self.url, url_variables = self.url_variables)
return url unless url.include?('{')
vars = url_variables
@@ -215,7 +215,19 @@ class WebHook < ApplicationRecord
end
def reset_url_variables
- self.url_variables = {} if url_changed? && !encrypted_url_variables_changed?
+ interpolated_url_was = interpolated_url(decrypt_url_was, url_variables_were)
+
+ return if url_variables_were.empty? || interpolated_url_was == interpolated_url
+
+ self.url_variables = {} if url_changed? && url_variables_were.to_a.intersection(url_variables.to_a).any?
+ end
+
+ def decrypt_url_was
+ self.class.decrypt_url(encrypted_url_was, iv: Base64.decode64(encrypted_url_iv_was))
+ end
+
+ def url_variables_were
+ self.class.decrypt_url_variables(encrypted_url_variables_was, iv: encrypted_url_variables_iv_was)
end
def next_failure_count
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index b85a57f81cd..cd05cc1763d 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -404,7 +404,6 @@ class ProjectPolicy < BasePolicy
end
rule { can?(:metrics_dashboard) }.policy do
- enable :read_prometheus
enable :read_deployment
end
diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb
index 711978dc3f7..66af1c087ed 100644
--- a/app/services/merge_requests/push_options_handler_service.rb
+++ b/app/services/merge_requests/push_options_handler_service.rb
@@ -54,7 +54,15 @@ module MergeRequests
end
def validate_service
- errors << 'User is required' if current_user.nil?
+ if current_user.nil?
+ errors << 'User is required'
+ return
+ end
+
+ unless current_user&.can?(:read_code, target_project)
+ errors << 'User access was denied'
+ return
+ end
unless target_project.merge_requests_enabled?
errors << "Merge requests are not enabled for project #{target_project.full_path}"
diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml
index fd807350245..543cbc4e4e4 100644
--- a/app/views/projects/tree/_tree_header.html.haml
+++ b/app/views/projects/tree/_tree_header.html.haml
@@ -2,7 +2,7 @@
.tree-ref-container.gl-display-flex.mb-2.mb-md-0
.tree-ref-holder
- #js-tree-ref-switcher{ data: { project_id: @project.id, project_root_path: project_path(@project) } }
+ #js-tree-ref-switcher{ data: { project_id: @project.id, ref_type: @ref_type.to_s, project_root_path: project_path(@project) } }
#js-repo-breadcrumb{ data: breadcrumb_data_attributes }
diff --git a/db/post_migrate/20230208131808_nullify_last_error_from_project_mirror_data.rb b/db/post_migrate/20230208131808_nullify_last_error_from_project_mirror_data.rb
new file mode 100644
index 00000000000..73e6f257498
--- /dev/null
+++ b/db/post_migrate/20230208131808_nullify_last_error_from_project_mirror_data.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+class NullifyLastErrorFromProjectMirrorData < Gitlab::Database::Migration[2.1]
+ MIGRATION = 'NullifyLastErrorFromProjectMirrorData'
+ INTERVAL = 2.minutes
+ BATCH_SIZE = 10_000
+ SUB_BATCH_SIZE = 1_000
+
+ disable_ddl_transaction!
+ restrict_gitlab_migration gitlab_schema: :gitlab_main
+
+ def up
+ queue_batched_background_migration(
+ MIGRATION,
+ :project_mirror_data,
+ :id,
+ job_interval: INTERVAL,
+ batch_size: BATCH_SIZE,
+ sub_batch_size: SUB_BATCH_SIZE
+ )
+ end
+
+ def down
+ delete_batched_background_migration(MIGRATION, :project_mirror_data, :id, [])
+ end
+end
diff --git a/db/schema_migrations/20230208131808 b/db/schema_migrations/20230208131808
new file mode 100644
index 00000000000..24c5b21f6ad
--- /dev/null
+++ b/db/schema_migrations/20230208131808
@@ -0,0 +1 @@
+784f8f189eee7b5cf3136f0a859874a1d170d2b148f4c260f968b144816f1322
\ No newline at end of file
diff --git a/glfm_specification/output_example_snapshots/snapshot_spec.html b/glfm_specification/output_example_snapshots/snapshot_spec.html
index 415ad1d0b11..73e1cd0a235 100644
--- a/glfm_specification/output_example_snapshots/snapshot_spec.html
+++ b/glfm_specification/output_example_snapshots/snapshot_spec.html
@@ -7028,7 +7028,7 @@ references and their corresponding code points.
-
<p> & © Æ Ď
+<p> & © Æ Ď
¾ ℋ ⅆ
∲ ≧̸</p>
@@ -7344,11 +7344,11 @@ stripped in this way:
-
<p><code> b </code></p>
+
<p><code> b </code></p>
@@ -7356,12 +7356,12 @@ stripped in this way:
-
<p><code> </code>
+<p><code> </code>
<code> </code></p>
@@ -7832,11 +7832,11 @@ not part of a [left-flanking delimiter run]:
-
<p>* a *</p>
+
<p>* a *</p>
@@ -9790,7 +9790,7 @@ Other [Unicode whitespace] like non-breaking space doesn't work.
-
[link](/url "title")
+
[link](/url "title")
diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb
index 70535496b12..6f8d34ea387 100644
--- a/lib/api/repositories.rb
+++ b/lib/api/repositories.rb
@@ -203,6 +203,10 @@ module API
render_api_error!("Target project id:#{params[:from_project_id]} is not a fork of project id:#{params[:id]}", 400)
end
+ unless can?(current_user, :read_code, target_project)
+ forbidden!("You don't have access to this fork's parent project")
+ end
+
cache_key = compare_cache_key(current_user, user_project, target_project, declared_params)
cache_action(cache_key, expires_in: 1.minute) do
diff --git a/lib/extracts_ref.rb b/lib/extracts_ref.rb
index f22996df0a5..23f93ad1317 100644
--- a/lib/extracts_ref.rb
+++ b/lib/extracts_ref.rb
@@ -5,7 +5,8 @@
# Can be extended for different types of repository object, e.g. Project or Snippet
module ExtractsRef
InvalidPathError = Class.new(StandardError)
-
+ BRANCH_REF_TYPE = 'heads'
+ TAG_REF_TYPE = 'tags'
# Given a string containing both a Git tree-ish, such as a branch or tag, and
# a filesystem path joined by forward slashes, attempts to separate the two.
#
@@ -91,7 +92,7 @@ module ExtractsRef
def ref_type
return unless params[:ref_type].present?
- params[:ref_type] == 'tags' ? 'tags' : 'heads'
+ params[:ref_type] == TAG_REF_TYPE ? TAG_REF_TYPE : BRANCH_REF_TYPE
end
private
@@ -154,4 +155,13 @@ module ExtractsRef
def repository_container
raise NotImplementedError
end
+
+ def ambiguous_ref?(project, ref)
+ return true if project.repository.ambiguous_ref?(ref)
+
+ return false unless ref&.starts_with?('refs/')
+
+ unprefixed_ref = ref.sub(%r{^refs/(heads|tags)/}, '')
+ project.repository.commit(unprefixed_ref).present?
+ end
end
diff --git a/lib/gitlab/background_migration/nullify_last_error_from_project_mirror_data.rb b/lib/gitlab/background_migration/nullify_last_error_from_project_mirror_data.rb
new file mode 100644
index 00000000000..6ea5c17353b
--- /dev/null
+++ b/lib/gitlab/background_migration/nullify_last_error_from_project_mirror_data.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module BackgroundMigration
+ # Nullifies last_error value from project_mirror_data table as they
+ # potentially included sensitive data.
+ # https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/3041
+ class NullifyLastErrorFromProjectMirrorData < BatchedMigrationJob
+ feature_category :source_code_management
+ operation_name :update_all
+
+ def perform
+ each_sub_batch { |rel| rel.update_all(last_error: nil) }
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/unicode.rb b/lib/gitlab/unicode.rb
index b49c5647dab..f291ea1b4ee 100644
--- a/lib/gitlab/unicode.rb
+++ b/lib/gitlab/unicode.rb
@@ -9,6 +9,12 @@ module Gitlab
# https://idiosyncratic-ruby.com/41-proper-unicoding.html
BIDI_REGEXP = /\p{Bidi Control}/.freeze
+ # Regular expression for identifying space characters
+ #
+ # In web browsers space characters can be confused with simple
+ # spaces which may be misleading
+ SPACE_REGEXP = /\p{Space_Separator}/.freeze
+
class << self
# Warning message used to highlight bidi characters in the GUI
def bidi_warning
diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb
index e3bf11b00b4..79e124a58f5 100644
--- a/lib/gitlab/url_sanitizer.rb
+++ b/lib/gitlab/url_sanitizer.rb
@@ -2,15 +2,37 @@
module Gitlab
class UrlSanitizer
+ include Gitlab::Utils::StrongMemoize
+
ALLOWED_SCHEMES = %w[http https ssh git].freeze
ALLOWED_WEB_SCHEMES = %w[http https].freeze
+ SCHEMIFIED_SCHEME = 'glschemelessuri'
+ SCHEMIFY_PLACEHOLDER = "#{SCHEMIFIED_SCHEME}://".freeze
+ # URI::DEFAULT_PARSER.make_regexp will only match URLs with schemes or
+ # relative URLs. This section will match schemeless URIs with userinfo
+ # e.g. user:pass@gitlab.com but will not match scp-style URIs e.g.
+ # user@server:path/to/file)
+ #
+ # The userinfo part is very loose compared to URI's implementation so we
+ # also match non-escaped userinfo e.g foo:b?r@gitlab.com which should be
+ # encoded as foo:b%3Fr@gitlab.com
+ URI_REGEXP = %r{
+ (?:
+ #{URI::DEFAULT_PARSER.make_regexp(ALLOWED_SCHEMES)}
+ |
+ (?:(?:(?!@)[%#{URI::REGEXP::PATTERN::UNRESERVED}#{URI::REGEXP::PATTERN::RESERVED}])+(?:@))
+ (?# negative lookahead ensures this isn't an SCP-style URL: [host]:[rel_path|abs_path] server:path/to/file)
+ (?!#{URI::REGEXP::PATTERN::HOST}:(?:#{URI::REGEXP::PATTERN::REL_PATH}|#{URI::REGEXP::PATTERN::ABS_PATH}))
+ #{URI::REGEXP::PATTERN::HOSTPORT}
+ )
+ }x
def self.sanitize(content)
- regexp = URI::DEFAULT_PARSER.make_regexp(ALLOWED_SCHEMES)
-
- content.gsub(regexp) { |url| new(url).masked_url }
- rescue Addressable::URI::InvalidURIError
- content.gsub(regexp, '')
+ content.gsub(URI_REGEXP) do |url|
+ new(url).masked_url
+ rescue Addressable::URI::InvalidURIError
+ ''
+ end
end
def self.valid?(url, allowed_schemes: ALLOWED_SCHEMES)
@@ -37,34 +59,45 @@ module Gitlab
@url = parse_url(url)
end
+ def credentials
+ @credentials ||= { user: @url.user.presence, password: @url.password.presence }
+ end
+
+ def user
+ credentials[:user]
+ end
+
def sanitized_url
- @sanitized_url ||= safe_url.to_s
+ safe_url = @url.dup
+ safe_url.password = nil
+ safe_url.user = nil
+ reverse_schemify(safe_url.to_s)
end
+ strong_memoize_attr :sanitized_url
def masked_url
url = @url.dup
url.password = "*****" if url.password.present?
url.user = "*****" if url.user.present?
- url.to_s
- end
-
- def credentials
- @credentials ||= { user: @url.user.presence, password: @url.password.presence }
- end
-
- def user
- credentials[:user]
+ reverse_schemify(url.to_s)
end
+ strong_memoize_attr :masked_url
def full_url
- @full_url ||= generate_full_url.to_s
+ return reverse_schemify(@url.to_s) unless valid_credentials?
+
+ url = @url.dup
+ url.password = encode_percent(credentials[:password]) if credentials[:password].present?
+ url.user = encode_percent(credentials[:user]) if credentials[:user].present?
+ reverse_schemify(url.to_s)
end
+ strong_memoize_attr :full_url
private
def parse_url(url)
- url = url.to_s.strip
- match = url.match(%r{\A(?:git|ssh|http(?:s?))\://(?:(.+)(?:@))?(.+)})
+ url = schemify(url.to_s.strip)
+ match = url.match(%r{\A(?:(?:#{SCHEMIFIED_SCHEME}|git|ssh|http(?:s?)):)?//(?:(.+)(?:@))?(.+)}o)
raw_credentials = match[1] if match
if raw_credentials.present?
@@ -83,24 +116,19 @@ module Gitlab
url
end
- def generate_full_url
- return @url unless valid_credentials?
-
- @url.dup.tap do |generated|
- generated.password = encode_percent(credentials[:password]) if credentials[:password].present?
- generated.user = encode_percent(credentials[:user]) if credentials[:user].present?
- end
+ def schemify(url)
+ # Prepend the placeholder scheme unless the URL has a scheme or is relative
+ url.prepend(SCHEMIFY_PLACEHOLDER) unless url.starts_with?(%r{(?:#{URI::REGEXP::PATTERN::SCHEME}:)?//}o)
+ url
end
- def safe_url
- safe_url = @url.dup
- safe_url.password = nil
- safe_url.user = nil
- safe_url
+ def reverse_schemify(url)
+ url.slice!(SCHEMIFY_PLACEHOLDER) if url.starts_with?(SCHEMIFY_PLACEHOLDER)
+ url
end
def valid_credentials?
- credentials && credentials.is_a?(Hash) && credentials.any?
+ credentials.is_a?(Hash) && credentials.values.any?
end
def encode_percent(string)
diff --git a/lib/rouge/formatters/html_gitlab.rb b/lib/rouge/formatters/html_gitlab.rb
index 436739bed12..a7e95a96b8b 100644
--- a/lib/rouge/formatters/html_gitlab.rb
+++ b/lib/rouge/formatters/html_gitlab.rb
@@ -25,7 +25,10 @@ module Rouge
yield %(
)
line.each do |token, value|
- yield highlight_unicode_control_characters(span(token, value.chomp! || value))
+ value = value.chomp! || value
+ value = replace_space_characters(value)
+
+ yield highlight_unicode_control_characters(span(token, value))
end
yield ellipsis if @ellipsis_indexes.include?(@line_number - 1) && @ellipsis_svg.present?
@@ -42,6 +45,10 @@ module Rouge
%(#{@ellipsis_svg})
end
+ def replace_space_characters(text)
+ text.gsub(Gitlab::Unicode::SPACE_REGEXP, ' ')
+ end
+
def highlight_unicode_control_characters(text)
text.gsub(Gitlab::Unicode::BIDI_REGEXP) do |char|
%(#{char})
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index ae06cb3ae7a..f8626094af5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -42005,6 +42005,9 @@ msgstr ""
msgid "The default branch for this project has been changed. Please update your bookmarks."
msgstr ""
+msgid "The default branch of this project clashes with another ref"
+msgstr ""
+
msgid "The dependency list details information about the components used within your project."
msgstr ""
diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb
index 4101bd7f658..4e68ffdda2a 100644
--- a/spec/controllers/admin/hooks_controller_spec.rb
+++ b/spec/controllers/admin/hooks_controller_spec.rb
@@ -59,6 +59,7 @@ RSpec.describe Admin::HooksController do
enable_ssl_verification: false,
url_variables: [
{ key: 'token', value: 'some secret value' },
+ { key: 'baz', value: 'qux' },
{ key: 'foo', value: nil }
]
}
@@ -71,7 +72,7 @@ RSpec.describe Admin::HooksController do
expect(flash[:notice]).to include('was updated')
expect(hook).to have_attributes(hook_params.except(:url_variables))
expect(hook).to have_attributes(
- url_variables: { 'token' => 'some secret value', 'baz' => 'woo' }
+ url_variables: { 'token' => 'some secret value', 'baz' => 'qux' }
)
end
end
diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb
index 334c156e1ae..b8a4b94aa66 100644
--- a/spec/controllers/concerns/confirm_email_warning_spec.rb
+++ b/spec/controllers/concerns/confirm_email_warning_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe ConfirmEmailWarning do
+RSpec.describe ConfirmEmailWarning, feature_category: :system_access do
before do
stub_feature_flags(soft_email_confirmation: true)
end
@@ -82,6 +82,38 @@ RSpec.describe ConfirmEmailWarning do
it { is_expected.to set_confirm_warning_for(user.email) }
end
end
+
+ context 'when user is being impersonated' do
+ let(:impersonator) { create(:admin) }
+
+ before do
+ allow(controller).to receive(:session).and_return({ impersonator_id: impersonator.id })
+
+ get :index
+ end
+
+ it { is_expected.to set_confirm_warning_for(user.email) }
+
+ context 'when impersonated user email has html in their email' do
+ let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: "malicious@test.com