Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2018-10-01 19:49:43 +0300
committerBob Van Landuyt <bob@vanlanduyt.co>2018-10-01 19:49:43 +0300
commit1cd07610664ab955c8a044b20c71224594a9a9bb (patch)
treee96e765ba0a148442efd477221fa9b77628f0f05
parentc874a481346d0cd83801a510135f29c72fd8d3ae (diff)
parent7cb9957a33d37394cd884106865e4aedef519e97 (diff)
Merge remote-tracking branch 'dev/master'
-rw-r--r--CHANGELOG.md36
-rw-r--r--app/assets/javascripts/issue_show/index.js7
-rw-r--r--app/assets/javascripts/pages/projects/issues/show.js3
-rw-r--r--app/controllers/projects/merge_requests/creations_controller.rb4
-rw-r--r--app/controllers/projects/merge_requests_controller.rb4
-rw-r--r--app/controllers/projects_controller.rb5
-rw-r--r--app/finders/events_finder.rb1
-rw-r--r--app/finders/joined_groups_finder.rb19
-rw-r--r--app/finders/user_recent_events_finder.rb1
-rw-r--r--app/mailers/emails/merge_requests.rb12
-rw-r--r--app/models/blob_viewer/package_json.rb3
-rw-r--r--app/models/event.rb14
-rw-r--r--app/models/group.rb30
-rw-r--r--app/models/merge_request.rb1
-rw-r--r--app/models/user.rb10
-rw-r--r--app/serializers/diff_line_entity.rb2
-rw-r--r--app/services/clusters/applications/check_installation_progress_service.rb6
-rw-r--r--app/services/clusters/applications/install_service.rb8
-rw-r--r--app/views/notify/new_merge_request_email.html.haml2
-rw-r--r--app/views/notify/new_merge_request_email.text.erb2
-rw-r--r--app/views/projects/merge_requests/_form.html.haml2
-rw-r--r--app/views/projects/merge_requests/creations/_new_submit.html.haml2
-rw-r--r--app/views/shared/issuable/_form.html.haml3
-rw-r--r--changelogs/unreleased/fix-events-finder-incomplete.yml5
-rw-r--r--changelogs/unreleased/security-2697-code-highlight-timeout.yml5
-rw-r--r--changelogs/unreleased/security-acet-issue-details.yml5
-rw-r--r--changelogs/unreleased/security-fj-stored-xss-in-repository-imports.yml5
-rw-r--r--changelogs/unreleased/security-package-json-xss.yml5
-rw-r--r--lib/api/events.rb22
-rw-r--r--lib/gitlab/diff/highlight.rb2
-rw-r--r--lib/gitlab/highlight.rb14
-rw-r--r--rubocop/cop/group_public_or_visible_to_user.rb22
-rw-r--r--rubocop/rubocop.rb1
-rw-r--r--spec/features/issues/issue_detail_spec.rb17
-rw-r--r--spec/javascripts/issue_show/index_spec.js19
-rw-r--r--spec/lib/gitlab/diff/highlight_spec.rb28
-rw-r--r--spec/lib/gitlab/highlight_spec.rb17
-rw-r--r--spec/models/blob_viewer/package_json_spec.rb21
-rw-r--r--spec/models/event_spec.rb124
-rw-r--r--spec/models/group_spec.rb42
-rw-r--r--spec/requests/api/groups_spec.rb14
-rw-r--r--spec/requests/api/redacted_events_spec.rb68
-rw-r--r--spec/rubocop/cop/group_public_or_visible_to_user_spec.rb28
-rw-r--r--spec/serializers/diff_line_entity_spec.rb45
-rw-r--r--spec/services/clusters/applications/check_installation_progress_service_spec.rb2
-rw-r--r--spec/services/clusters/applications/install_service_spec.rb2
-rw-r--r--spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb1
-rw-r--r--spec/views/projects/merge_requests/edit.html.haml_spec.rb1
48 files changed, 622 insertions, 70 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e514a42108c..c9ab8599d99 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,18 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
+## 11.3.1 (2018-09-26)
+
+### Security (6 changes)
+
+- Redact confidential events in the API.
+- Set timeout for syntax highlighting.
+- Sanitize JSON data properly to fix XSS on Issue details page.
+- Fix stored XSS in merge requests from imported repository.
+- Fix xss vulnerability sourced from package.json.
+- Block loopback addresses in UrlBlocker.
+
+
## 11.3.0 (2018-09-22)
### Security (5 changes, 1 of them is from the community)
@@ -249,6 +261,18 @@ entry.
- Creates Vue component for artifacts block on job page.
+## 11.2.4 (2018-09-26)
+
+### Security (6 changes)
+
+- Redact confidential events in the API.
+- Set timeout for syntax highlighting.
+- Sanitize JSON data properly to fix XSS on Issue details page.
+- Fix stored XSS in merge requests from imported repository.
+- Fix xss vulnerability sourced from package.json.
+- Block loopback addresses in UrlBlocker.
+
+
## 11.2.3 (2018-08-28)
### Fixed (1 change)
@@ -516,6 +540,18 @@ entry.
- Moves help_popover component to a common location.
+## 11.1.7 (2018-09-26)
+
+### Security (6 changes)
+
+- Redact confidential events in the API.
+- Set timeout for syntax highlighting.
+- Sanitize JSON data properly to fix XSS on Issue details page.
+- Fix stored XSS in merge requests from imported repository.
+- Fix xss vulnerability sourced from package.json.
+- Block loopback addresses in UrlBlocker.
+
+
## 11.1.6 (2018-08-28)
### Fixed (1 change)
diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js
index 75dfdedcf1b..d08e8ba0c4b 100644
--- a/app/assets/javascripts/issue_show/index.js
+++ b/app/assets/javascripts/issue_show/index.js
@@ -1,10 +1,11 @@
import Vue from 'vue';
+import sanitize from 'sanitize-html';
import issuableApp from './components/app.vue';
import '../vue_shared/vue_resource_interceptor';
-document.addEventListener('DOMContentLoaded', () => {
+export default function initIssueableApp() {
const initialDataEl = document.getElementById('js-issuable-app-initial-data');
- const props = JSON.parse(initialDataEl.innerHTML.replace(/&quot;/g, '"'));
+ const props = JSON.parse(sanitize(initialDataEl.textContent).replace(/&quot;/g, '"'));
return new Vue({
el: document.getElementById('js-issuable-app'),
@@ -17,4 +18,4 @@ document.addEventListener('DOMContentLoaded', () => {
});
},
});
-});
+}
diff --git a/app/assets/javascripts/pages/projects/issues/show.js b/app/assets/javascripts/pages/projects/issues/show.js
index 74b3a515e84..ef65196872c 100644
--- a/app/assets/javascripts/pages/projects/issues/show.js
+++ b/app/assets/javascripts/pages/projects/issues/show.js
@@ -3,9 +3,10 @@ import Issue from '~/issue';
import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable';
import ZenMode from '~/zen_mode';
import '~/notes/index';
-import '~/issue_show/index';
+import initIssueableApp from '~/issue_show';
export default function () {
+ initIssueableApp();
new Issue(); // eslint-disable-line no-new
new ShortcutsIssuable(); // eslint-disable-line no-new
new ZenMode(); // eslint-disable-line no-new
diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index 86583adc6a4..5639402a1e9 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -106,6 +106,10 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
@commits = set_commits_for_rendering(@merge_request.commits)
@commit = @merge_request.diff_head_commit
+ # FIXME: We have to assign a presenter to another instance variable
+ # due to class_name checks being made with issuable classes
+ @mr_presenter = @merge_request.present(current_user: current_user)
+
@labels = LabelsFinder.new(current_user, project_id: @project.id).execute
set_pipeline_variables
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index dfb69de650b..d691744d72a 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -333,6 +333,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@target_project = @merge_request.target_project
@target_branches = @merge_request.target_project.repository.branch_names
@noteable = @merge_request
+
+ # FIXME: We have to assign a presenter to another instance variable
+ # due to class_name checks being made with issuable classes
+ @mr_presenter = @merge_request.present(current_user: current_user)
end
def finder_type
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 7352c5e9bec..a9417369ca2 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -16,6 +16,7 @@ class ProjectsController < Projects::ApplicationController
before_action :tree, only: [:show], if: [:repo_exists?, :project_view_files?]
before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?]
before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
+ before_action :present_project, only: [:edit]
# Authorize
before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export]
@@ -433,4 +434,8 @@ class ProjectsController < Projects::ApplicationController
def whitelist_query_limiting
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42440')
end
+
+ def present_project
+ @project = @project.present(current_user: current_user)
+ end
end
diff --git a/app/finders/events_finder.rb b/app/finders/events_finder.rb
index fd7aeca0d8b..2e82bda8730 100644
--- a/app/finders/events_finder.rb
+++ b/app/finders/events_finder.rb
@@ -12,6 +12,7 @@ class EventsFinder
# Arguments:
# source - which user or project to looks for events on
# current_user - only return events for projects visible to this user
+ # WARNING: does not consider project feature visibility!
# params:
# action: string
# target_type: string
diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb
index 18cc6891ca4..4d8128dd824 100644
--- a/app/finders/joined_groups_finder.rb
+++ b/app/finders/joined_groups_finder.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true
-class JoinedGroupsFinder < UnionFinder
+class JoinedGroupsFinder
def initialize(user)
@user = user
end
@@ -8,19 +8,8 @@ class JoinedGroupsFinder < UnionFinder
# Finds the groups of the source user, optionally limited to those visible to
# the current user.
def execute(current_user = nil)
- segments = all_groups(current_user)
-
- find_union(segments, Group).order_id_desc
- end
-
- private
-
- def all_groups(current_user)
- groups = []
-
- groups << @user.authorized_groups.visible_to_user(current_user) if current_user
- groups << @user.authorized_groups.public_to_user(current_user)
-
- groups
+ @user.authorized_groups
+ .public_or_visible_to_user(current_user)
+ .order_id_desc
end
end
diff --git a/app/finders/user_recent_events_finder.rb b/app/finders/user_recent_events_finder.rb
index a4daf5b5841..eeca5026da1 100644
--- a/app/finders/user_recent_events_finder.rb
+++ b/app/finders/user_recent_events_finder.rb
@@ -3,6 +3,7 @@
# Get user activity feed for projects common for a user and a logged in user
#
# - current_user: The user viewing the events
+# WARNING: does not consider project feature visibility!
# - user: The user for which to load the events
# - params:
# - offset: The page of events to return
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index 67af0a4eb98..be085496731 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -3,13 +3,14 @@
module Emails
module MergeRequests
def new_merge_request_email(recipient_id, merge_request_id, reason = nil)
- setup_merge_request_mail(merge_request_id, recipient_id)
+ setup_merge_request_mail(merge_request_id, recipient_id, present: true)
mail_new_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id, reason))
end
def new_mention_in_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil)
- setup_merge_request_mail(merge_request_id, recipient_id)
+ setup_merge_request_mail(merge_request_id, recipient_id, present: true)
+
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
@@ -75,11 +76,16 @@ module Emails
private
- def setup_merge_request_mail(merge_request_id, recipient_id)
+ def setup_merge_request_mail(merge_request_id, recipient_id, present: false)
@merge_request = MergeRequest.find(merge_request_id)
@project = @merge_request.project
@target_url = project_merge_request_url(@project, @merge_request)
+ if present
+ recipient = User.find(recipient_id)
+ @mr_presenter = @merge_request.present(current_user: recipient)
+ end
+
@sent_notification = SentNotification.record(@merge_request, recipient_id, reply_key)
end
diff --git a/app/models/blob_viewer/package_json.rb b/app/models/blob_viewer/package_json.rb
index d12dd93ce2e..7cae60a74d6 100644
--- a/app/models/blob_viewer/package_json.rb
+++ b/app/models/blob_viewer/package_json.rb
@@ -33,7 +33,8 @@ module BlobViewer
end
def homepage
- json_data['homepage']
+ url = json_data['homepage']
+ url if Gitlab::UrlSanitizer.valid?(url)
end
def npm_url
diff --git a/app/models/event.rb b/app/models/event.rb
index 596155a9525..2e690f8c013 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -148,6 +148,8 @@ class Event < ActiveRecord::Base
end
end
+ # rubocop:disable Metrics/CyclomaticComplexity
+ # rubocop:disable Metrics/PerceivedComplexity
def visible_to_user?(user = nil)
if push? || commit_note?
Ability.allowed?(user, :download_code, project)
@@ -159,12 +161,18 @@ class Event < ActiveRecord::Base
Ability.allowed?(user, :read_issue, note? ? note_target : target)
elsif merge_request? || merge_request_note?
Ability.allowed?(user, :read_merge_request, note? ? note_target : target)
+ elsif personal_snippet_note?
+ Ability.allowed?(user, :read_personal_snippet, note_target)
+ elsif project_snippet_note?
+ Ability.allowed?(user, :read_project_snippet, note_target)
elsif milestone?
- Ability.allowed?(user, :read_project, project)
+ Ability.allowed?(user, :read_milestone, project)
else
false # No other event types are visible
end
end
+ # rubocop:enable Metrics/PerceivedComplexity
+ # rubocop:enable Metrics/CyclomaticComplexity
def project_name
if project
@@ -306,6 +314,10 @@ class Event < ActiveRecord::Base
note? && target && target.for_snippet?
end
+ def personal_snippet_note?
+ note? && target && target.for_personal_snippet?
+ end
+
def note_target
target.noteable
end
diff --git a/app/models/group.rb b/app/models/group.rb
index 62af20d2142..612c546ca57 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -82,8 +82,17 @@ class Group < Namespace
User.reference_pattern
end
- def visible_to_user(user)
- where(id: user.authorized_groups.select(:id).reorder(nil))
+ # WARNING: This method should never be used on its own
+ # please do make sure the number of rows you are filtering is small
+ # enough for this query
+ def public_or_visible_to_user(user)
+ return public_to_user unless user
+
+ public_for_user = public_to_user_arel(user)
+ visible_for_user = visible_to_user_arel(user)
+ public_or_visible = public_for_user.or(visible_for_user)
+
+ where(public_or_visible)
end
def select_for_project_authorization
@@ -95,6 +104,23 @@ class Group < Namespace
super
end
end
+
+ private
+
+ def public_to_user_arel(user)
+ self.arel_table[:visibility_level]
+ .in(Gitlab::VisibilityLevel.levels_for_user(user))
+ end
+
+ def visible_to_user_arel(user)
+ groups_table = self.arel_table
+ authorized_groups = user.authorized_groups.as('authorized')
+
+ groups_table.project(1)
+ .from(authorized_groups)
+ .where(authorized_groups[:id].eq(groups_table[:id]))
+ .exists
+ end
end
# Overrides notification_settings has_many association
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index dd5d494997d..0481a4a3d28 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -6,6 +6,7 @@ class MergeRequest < ActiveRecord::Base
include Issuable
include Noteable
include Referable
+ include Presentable
include IgnorableColumn
include TimeTrackable
include ManualInverseAssociation
diff --git a/app/models/user.rb b/app/models/user.rb
index eeac87e2e52..cd3b1c95b7e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -674,10 +674,12 @@ class User < ActiveRecord::Base
# Returns the groups a user has access to, either through a membership or a project authorization
def authorized_groups
- Group.from_union([
- groups,
- authorized_projects.joins(:namespace).select('namespaces.*')
- ])
+ Group.unscoped do
+ Group.from_union([
+ groups,
+ authorized_projects.joins(:namespace).select('namespaces.*')
+ ])
+ end
end
# Returns the groups a user is a member of, either directly or through a parent group
diff --git a/app/serializers/diff_line_entity.rb b/app/serializers/diff_line_entity.rb
index 2119a1017d3..942714b7787 100644
--- a/app/serializers/diff_line_entity.rb
+++ b/app/serializers/diff_line_entity.rb
@@ -9,6 +9,6 @@ class DiffLineEntity < Grape::Entity
expose :meta_positions, as: :meta_data
expose :rich_text do |line|
- line.rich_text || CGI.escapeHTML(line.text)
+ ERB::Util.html_escape(line.rich_text || line.text)
end
end
diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb
index 35f5cff0e0c..5017fa093f3 100644
--- a/app/services/clusters/applications/check_installation_progress_service.rb
+++ b/app/services/clusters/applications/check_installation_progress_service.rb
@@ -14,8 +14,8 @@ module Clusters
else
check_timeout
end
- rescue Kubeclient::HttpError => ke
- app.make_errored!("Kubernetes error: #{ke.message}") unless app.errored?
+ rescue Kubeclient::HttpError
+ app.make_errored!("Kubernetes error") unless app.errored?
end
private
@@ -27,7 +27,7 @@ module Clusters
end
def on_failed
- app.make_errored!(installation_errors || 'Installation silently failed')
+ app.make_errored!('Installation failed')
ensure
remove_installation_pod
end
diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb
index 7e3c0e77a83..dd8d2ed5eb6 100644
--- a/app/services/clusters/applications/install_service.rb
+++ b/app/services/clusters/applications/install_service.rb
@@ -12,10 +12,10 @@ module Clusters
ClusterWaitForAppInstallationWorker.perform_in(
ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id)
- rescue Kubeclient::HttpError => ke
- app.make_errored!("Kubernetes error: #{ke.message}")
- rescue StandardError => e
- app.make_errored!("Can't start installation process. #{e.message}")
+ rescue Kubeclient::HttpError
+ app.make_errored!("Kubernetes error.")
+ rescue StandardError
+ app.make_errored!("Can't start installation process.")
end
end
end
diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml
index dd6a84e503d..5acd45b74a7 100644
--- a/app/views/notify/new_merge_request_email.html.haml
+++ b/app/views/notify/new_merge_request_email.html.haml
@@ -9,7 +9,7 @@
%p
Assignee: #{@merge_request.assignee_name}
-= render_if_exists 'notify/merge_request_approvers', merge_request: @merge_request
+= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter
- if @merge_request.description
%div
diff --git a/app/views/notify/new_merge_request_email.text.erb b/app/views/notify/new_merge_request_email.text.erb
index d5b8f8d764f..754f4bca1cd 100644
--- a/app/views/notify/new_merge_request_email.text.erb
+++ b/app/views/notify/new_merge_request_email.text.erb
@@ -5,6 +5,6 @@ New Merge Request <%= @merge_request.to_reference %>
<%= merge_path_description(@merge_request, 'to') %>
Author: <%= @merge_request.author_name %>
Assignee: <%= @merge_request.assignee_name %>
-<%= render_if_exists 'notify/merge_request_approvers', merge_request: @merge_request %>
+<%= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter %>
<%= @merge_request.description %>
diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml
index 5a59f956cb5..13b967beba1 100644
--- a/app/views/projects/merge_requests/_form.html.haml
+++ b/app/views/projects/merge_requests/_form.html.haml
@@ -1,4 +1,4 @@
= form_for [@project.namespace.becomes(Namespace), @project, @merge_request],
html: { class: 'merge-request-form common-note-form js-requires-input js-quick-submit' },
data: { markdown_version: @merge_request.cached_markdown_version } do |f|
- = render 'shared/issuable/form', f: f, issuable: @merge_request
+ = render 'shared/issuable/form', f: f, issuable: @merge_request, presenter: @mr_presenter
diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml
index d5c4134dee2..464f8fa65e9 100644
--- a/app/views/projects/merge_requests/creations/_new_submit.html.haml
+++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml
@@ -11,7 +11,7 @@
= link_to 'Change branches', mr_change_branches_path(@merge_request)
%hr
= form_for [@project.namespace.becomes(Namespace), @project, @merge_request], html: { class: 'merge-request-form common-note-form js-requires-input js-quick-submit' } do |f|
- = render 'shared/issuable/form', f: f, issuable: @merge_request, commits: @commits
+ = render 'shared/issuable/form', f: f, issuable: @merge_request, commits: @commits, presenter: @mr_presenter
= f.hidden_field :source_project_id
= f.hidden_field :source_branch
= f.hidden_field :target_project_id
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 5b28a43a361..b33c758b464 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -1,6 +1,7 @@
- form = local_assigns.fetch(:f)
- commits = local_assigns[:commits]
- project = @target_project || @project
+- presenter = local_assigns.fetch(:presenter, nil)
= form_errors(issuable)
@@ -29,7 +30,7 @@
= render 'shared/issuable/form/metadata', issuable: issuable, form: form
-= render_if_exists 'shared/issuable/approvals', issuable: issuable, form: form
+= render_if_exists 'shared/issuable/approvals', issuable: issuable, presenter: presenter, form: form
= render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form
diff --git a/changelogs/unreleased/fix-events-finder-incomplete.yml b/changelogs/unreleased/fix-events-finder-incomplete.yml
new file mode 100644
index 00000000000..f3a4e421d33
--- /dev/null
+++ b/changelogs/unreleased/fix-events-finder-incomplete.yml
@@ -0,0 +1,5 @@
+---
+title: Redact confidential events in the API
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-2697-code-highlight-timeout.yml b/changelogs/unreleased/security-2697-code-highlight-timeout.yml
new file mode 100644
index 00000000000..66ad9ff822b
--- /dev/null
+++ b/changelogs/unreleased/security-2697-code-highlight-timeout.yml
@@ -0,0 +1,5 @@
+---
+title: Set timeout for syntax highlighting
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-acet-issue-details.yml b/changelogs/unreleased/security-acet-issue-details.yml
new file mode 100644
index 00000000000..64147a9d6e8
--- /dev/null
+++ b/changelogs/unreleased/security-acet-issue-details.yml
@@ -0,0 +1,5 @@
+---
+title: Sanitize JSON data properly to fix XSS on Issue details page
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-fj-stored-xss-in-repository-imports.yml b/changelogs/unreleased/security-fj-stored-xss-in-repository-imports.yml
new file mode 100644
index 00000000000..7520aa624c7
--- /dev/null
+++ b/changelogs/unreleased/security-fj-stored-xss-in-repository-imports.yml
@@ -0,0 +1,5 @@
+---
+title: Fix stored XSS in merge requests from imported repository
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/security-package-json-xss.yml b/changelogs/unreleased/security-package-json-xss.yml
new file mode 100644
index 00000000000..6ab4854e44f
--- /dev/null
+++ b/changelogs/unreleased/security-package-json-xss.yml
@@ -0,0 +1,5 @@
+---
+title: Fix xss vulnerability sourced from package.json
+merge_request:
+author:
+type: security
diff --git a/lib/api/events.rb b/lib/api/events.rb
index bca4db1a80e..6e0b508be19 100644
--- a/lib/api/events.rb
+++ b/lib/api/events.rb
@@ -18,12 +18,27 @@ module API
desc: 'Return events sorted in ascending and descending order'
end
+ RedactedEvent = OpenStruct.new(target_title: 'Confidential event').freeze
+
+ def redact_events(events)
+ events.map do |event|
+ if event.visible_to_user?(current_user)
+ event
+ else
+ RedactedEvent
+ end
+ end
+ end
+
# rubocop: disable CodeReuse/ActiveRecord
- def present_events(events)
+ def present_events(events, redact: true)
events = events.reorder(created_at: params[:sort])
.with_associations
- present paginate(events), with: Entities::Event
+ events = paginate(events)
+ events = redact_events(events) if redact
+
+ present events, with: Entities::Event
end
# rubocop: enable CodeReuse/ActiveRecord
end
@@ -46,7 +61,8 @@ module API
events = EventsFinder.new(params.merge(source: current_user, current_user: current_user)).execute.preload(:author, :target)
- present_events(events)
+ # Since we're viewing our own events, redaction is unnecessary
+ present_events(events, redact: false)
end
# rubocop: enable CodeReuse/ActiveRecord
end
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index 1f012043e56..a605ddb5c33 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -24,7 +24,7 @@ module Gitlab
# ignore highlighting for "match" lines
next diff_line if diff_line.meta?
- rich_line = highlight_line(diff_line) || diff_line.text
+ rich_line = highlight_line(diff_line) || ERB::Util.html_escape(diff_line.text)
if line_inline_diffs = inline_diffs[i]
begin
diff --git a/lib/gitlab/highlight.rb b/lib/gitlab/highlight.rb
index 5408a1a6838..0b6cc893db1 100644
--- a/lib/gitlab/highlight.rb
+++ b/lib/gitlab/highlight.rb
@@ -1,5 +1,8 @@
module Gitlab
class Highlight
+ TIMEOUT_BACKGROUND = 30.seconds
+ TIMEOUT_FOREGROUND = 3.seconds
+
def self.highlight(blob_name, blob_content, repository: nil, plain: false)
new(blob_name, blob_content, repository: repository)
.highlight(blob_content, continue: false, plain: plain)
@@ -51,11 +54,20 @@ module Gitlab
end
def highlight_rich(text, continue: true)
- @formatter.format(lexer.lex(text, continue: continue), tag: lexer.tag).html_safe
+ tag = lexer.tag
+ tokens = lexer.lex(text, continue: continue)
+ Timeout.timeout(timeout_time) { @formatter.format(tokens, tag: tag).html_safe }
+ rescue Timeout::Error => e
+ Gitlab::Sentry.track_exception(e)
+ highlight_plain(text)
rescue
highlight_plain(text)
end
+ def timeout_time
+ Sidekiq.server? ? TIMEOUT_BACKGROUND : TIMEOUT_FOREGROUND
+ end
+
def link_dependencies(text, highlighted_text)
Gitlab::DependencyLinker.link(blob_name, text, highlighted_text)
end
diff --git a/rubocop/cop/group_public_or_visible_to_user.rb b/rubocop/cop/group_public_or_visible_to_user.rb
new file mode 100644
index 00000000000..beda0b7f8ba
--- /dev/null
+++ b/rubocop/cop/group_public_or_visible_to_user.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+#
+module RuboCop
+ module Cop
+ # Cop that blacklists the usage of Group.public_or_visible_to_user
+ class GroupPublicOrVisibleToUser < RuboCop::Cop::Cop
+ MSG = '`Group.public_or_visible_to_user` should be used with extreme care. ' \
+ 'Please ensure that you are not using it on its own and that the amount ' \
+ 'of rows being filtered is reasonable.'
+
+ def_node_matcher :public_or_visible_to_user?, <<~PATTERN
+ (send (const nil? :Group) :public_or_visible_to_user ...)
+ PATTERN
+
+ def on_send(node)
+ return unless public_or_visible_to_user?(node)
+
+ add_offense(node, location: :expression)
+ end
+ end
+ end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index ff929c7b6ce..76d6037706e 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -38,3 +38,4 @@ require_relative 'cop/code_reuse/service_class'
require_relative 'cop/code_reuse/presenter'
require_relative 'cop/code_reuse/serializer'
require_relative 'cop/code_reuse/active_record'
+require_relative 'cop/group_public_or_visible_to_user'
diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb
index 088ab114df3..76bc93e9766 100644
--- a/spec/features/issues/issue_detail_spec.rb
+++ b/spec/features/issues/issue_detail_spec.rb
@@ -18,6 +18,23 @@ describe 'Issue Detail', :js do
end
end
+ context 'when issue description has xss snippet' do
+ before do
+ issue.update!(description: '![xss" onload=alert(1);//](a)')
+ sign_in(user)
+ visit project_issue_path(project, issue)
+ wait_for_requests
+ end
+
+ it 'should encode the description to prevent xss issues' do
+ page.within('.issuable-details .detail-page-description') do
+ expect(page).to have_selector('img', count: 1)
+ expect(find('img')['onerror']).to be_nil
+ expect(find('img')['src']).to end_with('/a')
+ end
+ end
+ end
+
context 'when edited by a user who is later deleted' do
before do
sign_in(user)
diff --git a/spec/javascripts/issue_show/index_spec.js b/spec/javascripts/issue_show/index_spec.js
new file mode 100644
index 00000000000..fa0b426c06c
--- /dev/null
+++ b/spec/javascripts/issue_show/index_spec.js
@@ -0,0 +1,19 @@
+import initIssueableApp from '~/issue_show';
+
+describe('Issue show index', () => {
+ describe('initIssueableApp', () => {
+ it('should initialize app with no potential XSS attack', () => {
+ const d = document.createElement('div');
+ d.id = 'js-issuable-app-initial-data';
+ d.innerHTML = JSON.stringify({
+ initialDescriptionHtml: '&lt;img src=x onerror=alert(1)&gt;',
+ });
+ document.body.appendChild(d);
+
+ const alertSpy = spyOn(window, 'alert');
+ initIssueableApp();
+
+ expect(alertSpy).not.toHaveBeenCalled();
+ });
+ });
+});
diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb
index 3c8cf9c56cc..5d0a603d11d 100644
--- a/spec/lib/gitlab/diff/highlight_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_spec.rb
@@ -8,6 +8,20 @@ describe Gitlab::Diff::Highlight do
let(:diff) { commit.raw_diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
+ shared_examples 'without inline diffs' do
+ let(:code) { '<h2 onmouseover="alert(2)">Test</h2>' }
+
+ before do
+ allow(Gitlab::Diff::InlineDiff).to receive(:for_lines).and_return([])
+ allow_any_instance_of(Gitlab::Diff::Line).to receive(:text).and_return(code)
+ end
+
+ it 'returns html escaped diff text' do
+ expect(subject[1].rich_text).to eq html_escape(code)
+ expect(subject[1].rich_text).to be_html_safe
+ end
+ end
+
describe '#highlight' do
context "with a diff file" do
let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
@@ -38,6 +52,16 @@ describe Gitlab::Diff::Highlight do
expect(subject[5].rich_text).to eq(code)
end
+
+ context 'when no diff_refs' do
+ before do
+ allow(diff_file).to receive(:diff_refs).and_return(nil)
+ end
+
+ context 'when no inline diffs' do
+ it_behaves_like 'without inline diffs'
+ end
+ end
end
context "with diff lines" do
@@ -93,6 +117,10 @@ describe Gitlab::Diff::Highlight do
expect { subject }. to raise_exception(RangeError)
end
end
+
+ context 'when no inline diffs' do
+ it_behaves_like 'without inline diffs'
+ end
end
end
end
diff --git a/spec/lib/gitlab/highlight_spec.rb b/spec/lib/gitlab/highlight_spec.rb
index 29e61d15726..88f7099ff3c 100644
--- a/spec/lib/gitlab/highlight_spec.rb
+++ b/spec/lib/gitlab/highlight_spec.rb
@@ -56,5 +56,22 @@ describe Gitlab::Highlight do
described_class.highlight('file.name', 'Contents')
end
+
+ context 'timeout' do
+ subject { described_class.new('file.name', 'Contents') }
+
+ it 'utilizes timeout for web' do
+ expect(Timeout).to receive(:timeout).with(described_class::TIMEOUT_FOREGROUND).and_call_original
+
+ subject.highlight("Content")
+ end
+
+ it 'utilizes longer timeout for sidekiq' do
+ allow(Sidekiq).to receive(:server?).and_return(true)
+ expect(Timeout).to receive(:timeout).with(described_class::TIMEOUT_BACKGROUND).and_call_original
+
+ subject.highlight("Content")
+ end
+ end
end
end
diff --git a/spec/models/blob_viewer/package_json_spec.rb b/spec/models/blob_viewer/package_json_spec.rb
index 5ed2f4400bc..fbaa8d47a71 100644
--- a/spec/models/blob_viewer/package_json_spec.rb
+++ b/spec/models/blob_viewer/package_json_spec.rb
@@ -40,13 +40,14 @@ describe BlobViewer::PackageJson do
end
context 'when package.json has "private": true' do
+ let(:homepage) { 'http://example.com' }
let(:data) do
<<-SPEC.strip_heredoc
{
"name": "module-name",
"version": "10.3.1",
"private": true,
- "homepage": "myawesomepackage.com"
+ "homepage": #{homepage.to_json}
}
SPEC
end
@@ -54,10 +55,22 @@ describe BlobViewer::PackageJson do
subject { described_class.new(blob) }
describe '#package_url' do
- it 'returns homepage if any' do
- expect(subject).to receive(:prepare!)
+ context 'when the homepage has a valid URL' do
+ it 'returns homepage URL' do
+ expect(subject).to receive(:prepare!)
+
+ expect(subject.package_url).to eq(homepage)
+ end
+ end
+
+ context 'when the homepage has an invalid URL' do
+ let(:homepage) { 'javascript:alert()' }
+
+ it 'returns nil' do
+ expect(subject).to receive(:prepare!)
- expect(subject.package_url).to eq('myawesomepackage.com')
+ expect(subject.package_url).to be_nil
+ end
end
end
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index c1eac4fa489..81748681528 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -148,9 +148,14 @@ describe Event do
let(:admin) { create(:admin) }
let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) }
+ let(:project_snippet) { create(:project_snippet, :public, project: project, author: author) }
+ let(:personal_snippet) { create(:personal_snippet, :public, author: author) }
let(:note_on_commit) { create(:note_on_commit, project: project) }
let(:note_on_issue) { create(:note_on_issue, noteable: issue, project: project) }
let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project) }
+ let(:note_on_project_snippet) { create(:note_on_project_snippet, author: author, noteable: project_snippet, project: project) }
+ let(:note_on_personal_snippet) { create(:note_on_personal_snippet, author: author, noteable: personal_snippet, project: nil) }
+ let(:milestone_on_project) { create(:milestone, project: project) }
let(:event) { described_class.new(project: project, target: target, author_id: author.id) }
before do
@@ -268,6 +273,125 @@ describe Event do
end
end
end
+
+ context 'milestone event' do
+ let(:target) { milestone_on_project }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_truthy
+ expect(event.visible_to_user?(non_member)).to be_truthy
+ expect(event.visible_to_user?(member)).to be_truthy
+ expect(event.visible_to_user?(guest)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+
+ context 'on public project with private issue tracker and merge requests' do
+ let(:project) { create(:project, :public, :issues_private, :merge_requests_private) }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_falsy
+ expect(event.visible_to_user?(non_member)).to be_falsy
+ expect(event.visible_to_user?(member)).to be_truthy
+ expect(event.visible_to_user?(guest)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+ end
+
+ context 'on private project' do
+ let(:project) { create(:project, :private) }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_falsy
+ expect(event.visible_to_user?(non_member)).to be_falsy
+ expect(event.visible_to_user?(member)).to be_truthy
+ expect(event.visible_to_user?(guest)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+ end
+ end
+
+ context 'project snippet note event' do
+ let(:target) { note_on_project_snippet }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_truthy
+ expect(event.visible_to_user?(non_member)).to be_truthy
+ expect(event.visible_to_user?(author)).to be_truthy
+ expect(event.visible_to_user?(member)).to be_truthy
+ expect(event.visible_to_user?(guest)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+
+ context 'on public project with private snippets' do
+ let(:project) { create(:project, :public, :snippets_private) }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_falsy
+ expect(event.visible_to_user?(non_member)).to be_falsy
+
+ # Normally, we'd expect the author of a comment to be able to view it.
+ # However, this doesn't seem to be the case for comments on snippets.
+ expect(event.visible_to_user?(author)).to be_falsy
+
+ expect(event.visible_to_user?(member)).to be_truthy
+ expect(event.visible_to_user?(guest)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+ end
+
+ context 'on private project' do
+ let(:project) { create(:project, :private) }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_falsy
+ expect(event.visible_to_user?(non_member)).to be_falsy
+
+ # Normally, we'd expect the author of a comment to be able to view it.
+ # However, this doesn't seem to be the case for comments on snippets.
+ expect(event.visible_to_user?(author)).to be_falsy
+
+ expect(event.visible_to_user?(member)).to be_truthy
+ expect(event.visible_to_user?(guest)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+ end
+ end
+
+ context 'personal snippet note event' do
+ let(:target) { note_on_personal_snippet }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_truthy
+ expect(event.visible_to_user?(non_member)).to be_truthy
+ expect(event.visible_to_user?(author)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+
+ context 'on internal snippet' do
+ let(:personal_snippet) { create(:personal_snippet, :internal, author: author) }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_falsy
+ expect(event.visible_to_user?(non_member)).to be_truthy
+ expect(event.visible_to_user?(author)).to be_truthy
+ expect(event.visible_to_user?(admin)).to be_truthy
+ end
+ end
+
+ context 'on private snippet' do
+ let(:personal_snippet) { create(:personal_snippet, :private, author: author) }
+
+ it do
+ expect(event.visible_to_user?(nil)).to be_falsy
+ expect(event.visible_to_user?(non_member)).to be_falsy
+ expect(event.visible_to_user?(author)).to be_truthy
+
+ # It is very unexpected that a private personal snippet is not visible
+ # to an instance administrator. This should be fixed in the future.
+ expect(event.visible_to_user?(admin)).to be_falsy
+ end
+ end
+ end
end
describe '.limit_recent' do
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 0729eb99e78..1bf8f89e126 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -169,22 +169,42 @@ describe Group do
end
end
- describe '.visible_to_user' do
- let!(:group) { create(:group) }
- let!(:user) { create(:user) }
+ describe '.public_or_visible_to_user' do
+ let!(:private_group) { create(:group, :private) }
+ let!(:internal_group) { create(:group, :internal) }
- subject { described_class.visible_to_user(user) }
+ subject { described_class.public_or_visible_to_user(user) }
- describe 'when the user has access to a group' do
- before do
- group.add_user(user, Gitlab::Access::MAINTAINER)
- end
+ context 'when user is nil' do
+ let!(:user) { nil }
- it { is_expected.to eq([group]) }
+ it { is_expected.to match_array([group]) }
end
- describe 'when the user does not have access to any groups' do
- it { is_expected.to eq([]) }
+ context 'when user' do
+ let!(:user) { create(:user) }
+
+ context 'when user does not have access to any private group' do
+ it { is_expected.to match_array([internal_group, group]) }
+ end
+
+ context 'when user is a member of private group' do
+ before do
+ private_group.add_user(user, Gitlab::Access::DEVELOPER)
+ end
+
+ it { is_expected.to match_array([private_group, internal_group, group]) }
+ end
+
+ context 'when user is a member of private subgroup', :postgresql do
+ let!(:private_subgroup) { create(:group, :private, parent: private_group) }
+
+ before do
+ private_subgroup.add_user(user, Gitlab::Access::DEVELOPER)
+ end
+
+ it { is_expected.to match_array([private_subgroup, internal_group, group]) }
+ end
end
end
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index 3a8948f8477..3802b5c6848 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -155,7 +155,7 @@ describe API::Groups do
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
- expect(response_groups).to eq(Group.visible_to_user(user1).order(:name).pluck(:name))
+ expect(response_groups).to eq(groups_visible_to_user(user1).order(:name).pluck(:name))
end
it "sorts in descending order when passed" do
@@ -164,7 +164,7 @@ describe API::Groups do
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
- expect(response_groups).to eq(Group.visible_to_user(user1).order(name: :desc).pluck(:name))
+ expect(response_groups).to eq(groups_visible_to_user(user1).order(name: :desc).pluck(:name))
end
it "sorts by path in order_by param" do
@@ -173,7 +173,7 @@ describe API::Groups do
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
- expect(response_groups).to eq(Group.visible_to_user(user1).order(:path).pluck(:name))
+ expect(response_groups).to eq(groups_visible_to_user(user1).order(:path).pluck(:name))
end
it "sorts by id in the order_by param" do
@@ -182,7 +182,7 @@ describe API::Groups do
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
- expect(response_groups).to eq(Group.visible_to_user(user1).order(:id).pluck(:name))
+ expect(response_groups).to eq(groups_visible_to_user(user1).order(:id).pluck(:name))
end
it "sorts also by descending id with pagination fix" do
@@ -191,7 +191,7 @@ describe API::Groups do
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
- expect(response_groups).to eq(Group.visible_to_user(user1).order(id: :desc).pluck(:name))
+ expect(response_groups).to eq(groups_visible_to_user(user1).order(id: :desc).pluck(:name))
end
it "sorts identical keys by id for good pagination" do
@@ -211,6 +211,10 @@ describe API::Groups do
expect(json_response).to be_an Array
expect(response_groups_ids).to eq(Group.select { |group| group['name'] == 'same-name' }.map { |group| group['id'] }.sort)
end
+
+ def groups_visible_to_user(user)
+ Group.where(id: user.authorized_groups.select(:id).reorder(nil))
+ end
end
context 'when using owned in the request' do
diff --git a/spec/requests/api/redacted_events_spec.rb b/spec/requests/api/redacted_events_spec.rb
new file mode 100644
index 00000000000..086dd3df9ba
--- /dev/null
+++ b/spec/requests/api/redacted_events_spec.rb
@@ -0,0 +1,68 @@
+require 'spec_helper'
+
+describe 'Redacted events in API::Events' do
+ shared_examples 'private events are redacted' do
+ it 'redacts events the user does not have access to' do
+ expect_any_instance_of(Event).to receive(:visible_to_user?).and_call_original
+
+ get api(path), user
+
+ expect(response).to have_gitlab_http_status(200)
+ expect(json_response).to contain_exactly(
+ 'project_id' => nil,
+ 'action_name' => nil,
+ 'target_id' => nil,
+ 'target_iid' => nil,
+ 'target_type' => nil,
+ 'author_id' => nil,
+ 'target_title' => 'Confidential event',
+ 'created_at' => nil,
+ 'author_username' => nil
+ )
+ end
+ end
+
+ describe '/users/:id/events' do
+ let(:project) { create(:project, :public) }
+ let(:path) { "/users/#{project.owner.id}/events" }
+ let(:issue) { create(:issue, :confidential, project: project) }
+
+ before do
+ EventCreateService.new.open_issue(issue, issue.author)
+ end
+
+ context 'unauthenticated user views another user with private events' do
+ let(:user) { nil }
+
+ include_examples 'private events are redacted'
+ end
+
+ context 'authenticated user without access views another user with private events' do
+ let(:user) { create(:user) }
+
+ include_examples 'private events are redacted'
+ end
+ end
+
+ describe '/projects/:id/events' do
+ let(:project) { create(:project, :public) }
+ let(:path) { "/projects/#{project.id}/events" }
+ let(:issue) { create(:issue, :confidential, project: project) }
+
+ before do
+ EventCreateService.new.open_issue(issue, issue.author)
+ end
+
+ context 'unauthenticated user views public project' do
+ let(:user) { nil }
+
+ include_examples 'private events are redacted'
+ end
+
+ context 'authenticated user without access views public project' do
+ let(:user) { create(:user) }
+
+ include_examples 'private events are redacted'
+ end
+ end
+end
diff --git a/spec/rubocop/cop/group_public_or_visible_to_user_spec.rb b/spec/rubocop/cop/group_public_or_visible_to_user_spec.rb
new file mode 100644
index 00000000000..7b5235a3da7
--- /dev/null
+++ b/spec/rubocop/cop/group_public_or_visible_to_user_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/group_public_or_visible_to_user'
+
+describe RuboCop::Cop::GroupPublicOrVisibleToUser do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'flags the use of Group.public_or_visible_to_user with a constant receiver' do
+ inspect_source('Group.public_or_visible_to_user')
+
+ expect(cop.offenses.size).to eq(1)
+ end
+
+ it 'does not flat the use of public_or_visible_to_user with a constant that is not Group' do
+ inspect_source('Project.public_or_visible_to_user')
+
+ expect(cop.offenses.size).to eq(0)
+ end
+
+ it 'does not flag the use of Group.public_or_visible_to_user with a send receiver' do
+ inspect_source('foo.public_or_visible_to_user')
+
+ expect(cop.offenses.size).to eq(0)
+ end
+end
diff --git a/spec/serializers/diff_line_entity_spec.rb b/spec/serializers/diff_line_entity_spec.rb
new file mode 100644
index 00000000000..2549f64bcd3
--- /dev/null
+++ b/spec/serializers/diff_line_entity_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DiffLineEntity do
+ include RepoHelpers
+
+ let(:code) { 'hello world' }
+ let(:line) { Gitlab::Diff::Line.new(code, 'new', 1, nil, 1) }
+ let(:entity) { described_class.new(line, request: {}) }
+
+ subject { entity.as_json }
+
+ it 'exposes correct attributes' do
+ expect(subject).to include(
+ :line_code, :type, :old_line, :new_line, :text, :meta_data, :rich_text
+ )
+ end
+
+ describe '#rich_text' do
+ let(:code) { '<h2 onmouseover="alert(2)">Test</h2>' }
+ let(:rich_text_value) { nil }
+
+ before do
+ line.instance_variable_set(:@rich_text, rich_text_value)
+ end
+
+ shared_examples 'escapes html tags' do
+ it do
+ expect(subject[:rich_text]).to eq html_escape(code)
+ expect(subject[:rich_text]).to be_html_safe
+ end
+ end
+
+ context 'when rich_line is present' do
+ let(:rich_text_value) { code }
+
+ it_behaves_like 'escapes html tags'
+ end
+
+ context 'when rich_line is not present' do
+ it_behaves_like 'escapes html tags'
+ end
+ end
+end
diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb
index 986f11410fd..1a565bb734d 100644
--- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb
+++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb
@@ -82,7 +82,7 @@ describe Clusters::Applications::CheckInstallationProgressService do
service.execute
expect(application).to be_errored
- expect(application.status_reason).to eq(errors)
+ expect(application.status_reason).to eq("Installation failed")
end
end
diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb
index a744ec30b65..4bd19f5bd79 100644
--- a/spec/services/clusters/applications/install_service_spec.rb
+++ b/spec/services/clusters/applications/install_service_spec.rb
@@ -42,7 +42,7 @@ describe Clusters::Applications::InstallService do
service.execute
expect(application).to be_errored
- expect(application.status_reason).to match(/kubernetes error:/i)
+ expect(application.status_reason).to match('Kubernetes error.')
end
end
diff --git a/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb b/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb
index 8befae39d3a..0206928a211 100644
--- a/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb
+++ b/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb
@@ -12,6 +12,7 @@ describe 'projects/merge_requests/creations/_new_submit.html.haml' do
assign(:hidden_commit_count, 0)
assign(:total_commit_count, merge_request.commits.count)
assign(:project, merge_request.target_project)
+ assign(:mr_presenter, merge_request.present(current_user: merge_request.author))
allow(view).to receive(:can?).and_return(true)
allow(view).to receive(:url_for).and_return('#')
diff --git a/spec/views/projects/merge_requests/edit.html.haml_spec.rb b/spec/views/projects/merge_requests/edit.html.haml_spec.rb
index 9b74a7e1946..c13eab30054 100644
--- a/spec/views/projects/merge_requests/edit.html.haml_spec.rb
+++ b/spec/views/projects/merge_requests/edit.html.haml_spec.rb
@@ -24,6 +24,7 @@ describe 'projects/merge_requests/edit.html.haml' do
before do
assign(:project, project)
assign(:merge_request, closed_merge_request)
+ assign(:mr_presenter, closed_merge_request.present(current_user: user))
allow(view).to receive(:can?).and_return(true)
allow(view).to receive(:current_user)